Re: [HACKERS] Time to change pg_regress diffs to unified by default?

[Tom Lane]

Peter Eisentraut  writes:
> I think one problem is that diff -u is not as portable as diff -c.  For
> example, the HP-UX 11 man page of diff doesn't list it.

FWIW, I can confirm that HPUX 10.20's diff hasn't got it.  That would
not affect gaur/pademelon, if we make this change, because I installed
GNU diffutils on that machine a decade or two ago.  It might be a bigger
issue for the other HPUX critters though.

Some other data points:

* POSIX 2008 requires diff -u.
* SUS v2 (POSIX 1997) does not.
* My other pet dinosaur, prairiedog (macOS 10.4 something), has it.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Time to change pg_regress diffs to unified by default?

[Noah Misch]

On Thu, Apr 06, 2017 at 07:01:32PM -0400, Tom Lane wrote:
> Andres Freund  writes:
> > I personally, and I know of a bunch of other regular contributors, find
> > context diffs very hard to read.  Besides general dislike, for things
> > like regression test output context diffs are just not well suited.
> 
> Personally, I disagree completely.  Unified diffs are utterly unreadable
> for anything beyond trivial cases of small well-separated changes.
> 
> It's possible that regression failure diffs will usually fall into that
> category, but I'm not convinced.

For reading patches, I frequently use both formats.  Overall, I perhaps read
unified 3/4 of the time and context 1/4 of the time.

For regression diffs, I use PG_REGRESS_DIFF_OPTS=-u and have never converted a
regression diff to context form.  Hence, +1 for the proposed change.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Time to change pg_regress diffs to unified by default?

[David Rowley]

On 7 April 2017 at 10:31, Andres Freund  wrote:
> Hi,
>
> I personally, and I know of a bunch of other regular contributors, find
> context diffs very hard to read.  Besides general dislike, for things
> like regression test output context diffs are just not well suited.
> E.g. in
> https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=prairiedog=2017-04-06%2021%3A10%3A56=check
> the salient point (ERROR:  50 is outside the valid range for parameter 
> "effective_io_concurrency" (0 .. 0))
> is 130 lines into the diff, whereas it's right at the start in a unified diff.
> Issues with one error that causes a lot of followup output changes are
> really common in our regression suite.
>
> I personally have PG_REGRESS_DIFF_OPTS set to -dU10, but that doesn't
> help much analyzing buildfarm output.
>
> Therefore I propose changing the defaults in pg_regress.c.

You mean people actually use those diffs? I've never done anything
apart from using . That way I can
reject and accept the changes as I wish, just by kicking the results
over to the expected results, or not, if there's a genuine mistake.


-- 
 David Rowley   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Time to change pg_regress diffs to unified by default?

[Andrew Dunstan]

On 04/06/2017 09:17 PM, Peter Eisentraut wrote:
> On 4/6/17 18:31, Andres Freund wrote:
>> I personally have PG_REGRESS_DIFF_OPTS set to -dU10, but that doesn't
>> help much analyzing buildfarm output.
>>
>> Therefore I propose changing the defaults in pg_regress.c.
> I think one problem is that diff -u is not as portable as diff -c.  For
> example, the HP-UX 11 man page of diff doesn't list it.
>


Ugh. I suppose we could run a test to see if it was available. If it
comes to that, I guess I could do a similar test in the buildfarm
client. Seems a bit like overkill, though.

cheers

andrew

-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Time to change pg_regress diffs to unified by default?

[Peter Eisentraut]

On 4/6/17 18:31, Andres Freund wrote:
> I personally have PG_REGRESS_DIFF_OPTS set to -dU10, but that doesn't
> help much analyzing buildfarm output.
> 
> Therefore I propose changing the defaults in pg_regress.c.

I think one problem is that diff -u is not as portable as diff -c.  For
example, the HP-UX 11 man page of diff doesn't list it.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Time to change pg_regress diffs to unified by default?

[Tom Lane]

Andres Freund  writes:
> I personally, and I know of a bunch of other regular contributors, find
> context diffs very hard to read.  Besides general dislike, for things
> like regression test output context diffs are just not well suited.

Personally, I disagree completely.  Unified diffs are utterly unreadable
for anything beyond trivial cases of small well-separated changes.

It's possible that regression failure diffs will usually fall into that
category, but I'm not convinced.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Time to change pg_regress diffs to unified by default?

[Andrew Dunstan]

On 04/06/2017 06:31 PM, Andres Freund wrote:
> Hi,
>
> I personally, and I know of a bunch of other regular contributors, find
> context diffs very hard to read.  Besides general dislike, for things
> like regression test output context diffs are just not well suited.
> E.g. in
> https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=prairiedog=2017-04-06%2021%3A10%3A56=check
> the salient point (ERROR:  50 is outside the valid range for parameter 
> "effective_io_concurrency" (0 .. 0))
> is 130 lines into the diff, whereas it's right at the start in a unified diff.
> Issues with one error that causes a lot of followup output changes are
> really common in our regression suite.
>
> I personally have PG_REGRESS_DIFF_OPTS set to -dU10, but that doesn't
> help much analyzing buildfarm output.
>
> Therefore I propose changing the defaults in pg_regress.c.
>

+1

cheers

andrew

-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Time to change pg_regress diffs to unified by default?

[Thomas Munro]

On Fri, Apr 7, 2017 at 10:31 AM, Andres Freund  wrote:
> Hi,
>
> I personally, and I know of a bunch of other regular contributors, find
> context diffs very hard to read.  Besides general dislike, for things
> like regression test output context diffs are just not well suited.
> E.g. in
> https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=prairiedog=2017-04-06%2021%3A10%3A56=check
> the salient point (ERROR:  50 is outside the valid range for parameter 
> "effective_io_concurrency" (0 .. 0))
> is 130 lines into the diff, whereas it's right at the start in a unified diff.
> Issues with one error that causes a lot of followup output changes are
> really common in our regression suite.
>
> I personally have PG_REGRESS_DIFF_OPTS set to -dU10, but that doesn't
> help much analyzing buildfarm output.
>
> Therefore I propose changing the defaults in pg_regress.c.

+1

So much easier to read.

-- 
Thomas Munro
http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Time to change pg_regress diffs to unified by default?

[Andres Freund]

Hi,

I personally, and I know of a bunch of other regular contributors, find
context diffs very hard to read.  Besides general dislike, for things
like regression test output context diffs are just not well suited.
E.g. in
https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=prairiedog=2017-04-06%2021%3A10%3A56=check
the salient point (ERROR:  50 is outside the valid range for parameter 
"effective_io_concurrency" (0 .. 0))
is 130 lines into the diff, whereas it's right at the start in a unified diff.
Issues with one error that causes a lot of followup output changes are
really common in our regression suite.

I personally have PG_REGRESS_DIFF_OPTS set to -dU10, but that doesn't
help much analyzing buildfarm output.

Therefore I propose changing the defaults in pg_regress.c.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers