Thread: [PATCH] Regression tests in windows ignore white space

On Thu, Dec 26, 2013 at 3:02 PM, David Rowley <dgrowleyml@gmail.com> wrote:
> In the following thread I discovered that my new regression tests worked
> perfectly on windows, but when they were run on linux they failed.
>
> http://www.postgresql.org/message-id/CAApHDvo_YCiPYGDz07MpX9o6EGg=3mmyJTb0ysPTwoTg3c=Tvw@mail.gmail.com
>
> After looking at pg_regress to see which options it passes to diff I
> discovered that it passes -w on windows to ignore ALL white space.
>
> The attached simple patch changes this so that it only ignores carriage
> returns. It does this by passing --strip-trailing-cr to diff instead of -w.
> This should help us few developers who use windows to get our white space
> correct in out expected results so that the tests also pass on non windows
> platforms.
>

I also faced some similar issue few days back and tried to search a
bit for some simpler option, but couldn't spend too much time.
I think it would be good if we can make it work without -w option.

When I tried with your patch on windows, it results into following:

============== running regression test queries        ==============
test tablespace               ... diff: unrecognized option `--strip-trailing-cr
'
diff: Try `diff --help' for more information.
diff command failed with status 2: "diff --strip-trailing-cr
"e:/../postgresql/src/test/regress/expected/tablespace.out"
"e:/../postgresql/src/test/regress/results/tablespace.out" >
"e:/../postgresql/src/test/regress/results/tablespace.out.diff"
"

Which version of diff you are using?

Version of diff on my m/c is:
diff - GNU diffutils version 2.7

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On Sun, Dec 29, 2013 at 2:06 AM, David Rowley <dgrowleyml@gmail.com> wrote:
> On Sun, Dec 29, 2013 at 2:29 AM, Amit Kapila <amit.kapila16@gmail.com>
> wrote:
>>
>> When I tried with your patch on windows, it results into following:
>>
>> ============== running regression test queries        ==============
>> test tablespace               ... diff: unrecognized option
>> `--strip-trailing-cr
>> '
>> Which version of diff you are using?
>>
>> Version of diff on my m/c is:
>> diff - GNU diffutils version 2.7
>>
>
> I had a bit of a look around on the git repository for diffutils and I've
> found at least part of the commit which introduced --strip-trailing-cr
>
> http://git.savannah.gnu.org/cgit/diffutils.git/commit/?id=eefb9adae1642dcb0e2ac523c79998f466e94e77
>
> Although here I can only see that they've added the command line args and
> not the actual code which implements stripping the carriage returns. Going
> by that it seems that was added back in 2001, but the version you're using
> is a bit older than than, it seems 2.7 is 19 years old!

For Windows build, I am using whatever latest Git provides rather than
downloading
individual components which might not be good, but I find it
convenient. The latest
Git (1.8.4) download on windows still provides 2.7, which is the
reason I am on older
version. However I agree that it is better to use latest version.

> http://git.savannah.gnu.org/cgit/diffutils.git/refs/tags
>
> I'm on 2.8.7 which is only 4 years old.
>
> I know we don't normally go with bleeding edge, but I wondering if we could
> make the minimum supported diff version at least 2.8 (at least for windows)
> which was released in 2002.

I have checked that for some of the other components like bison, flex,
ActiveState TCL we specify minimum version required, but not for diff.
http://www.postgresql.org/docs/devel/static/install-windows-full.html

+1, for minimum diff version as 2.8.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On Mon, Dec 30, 2013 at 11:08 AM, David Rowley <dgrowleyml@gmail.com> wrote:
> On Mon, Dec 30, 2013 at 6:02 PM, Amit Kapila <amit.kapila16@gmail.com>
> wrote:
>> For Windows build, I am using whatever latest Git provides rather than
>> downloading
>> individual components which might not be good, but I find it
>> convenient. The latest
>> Git (1.8.4) download on windows still provides 2.7, which is the
>> reason I am on older
>> version. However I agree that it is better to use latest version.
>
>
> It looks like the diff version I'm using is from msys and msys is what is in
> my search path rather than the git\bin path. To be honest I didn't realise
> that git for windows came with bison and flex, (at least I see bison.exe and
> flex.exe in the git\bin path.. I don't remember me putting them there)
  Yes, git for windows came with bison,flex, etc required for PG build.

> I don't seem to even have git\bin in my %path% environment variable at all,
> so all those tools are being picked up from the msys path. I'd need to
> remind myself about the msys install process, but since we're already saying
> in the docs that msys should be installed,  I think it is one of the ways of set up on Windows, not every user
uses from msys.

> then would the fix for this not
> just be as simple as my patch plus a note in the docs to say to ensure
> msys\bin occurs before git\bin in the %path% environment var, minimum
> supported diff version is 2.8.
 To be honest, I am also not fully aware of the project policy in such a matter where we have to mention the minimum
versionof dependent executable (diff). I request some senior members to please let us know whether we can make PG
dependenton diff version >= 2.8 (atleast for windows), as there are some of the options which are not available in
olderversions.
 
 Another option could be that if version of diff available is >= 2.8, then use '--strip-trailing-cr', else use existing
option'-w', but not
 
sure if it will serve the purpose completely.

> Did you install msys? if so does it have a later version of diff?  No.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com