Appveyor: patching of Mkvcbuild.pm and vcregress.pl, should happen automatically about plr HOT 11 CLOSED

Cool, thanks for this. So the reason master is failing is because for some reason the error is now returning a warning on windows but not on linux

-ERROR:  R interpreter parse error
+WARNING:  R interpreter parse error
 DETAIL:  R parse error caught in "{
   deadbeef <- function(,bad) {}
   42

we could use the same thing and change the ERROR to WARNING in the expected out files
rather you could as am not that familiar with writing in R :)

from plr.

Dave,

Thanks. O.K. I see, now!

When, where, and why, are now concerning me.

from plr.

Dave,

Also, I will do some work (brute force mostly), trying to track down where, that change occurred.

from plr.

I doubt R changed things, so I am wondering if this on postgres side?

from plr.

Dave,

I tested with R 3.6.0 and master, still the same ERROR/WARNING problem occurs. So yes, I agree the problem, does not seem to be on the R side.

I tried testing PostgreSQL master commits at 6/30, 9/30, and 12/30, however none of them built: They failed for non-ERROR/WARNING reasons. Maybe master has some untested commits? For the moment, (So, a brute force bisection method will take a while).

Currently, I am giving up trying to figure out where/when the ERROR/WARNING started happening.

Short story, as you recommended: I patched around the problem, and the regression test now passes. I patched around it, using simple "sed", (I did not need R in this case).

Build matrix:
https://github.com/AndreMikulec/plr/blob/237812c25c813b3a64b07a1c36fd4fa90bce29ce/appveyor.yml#L21

  - pg: master # branch - non-static commit - from git
    PlatformToolset: v141
    configuration: Debug
    platform: x64
    APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
    rversion: 4.0.5

If building from source, Tito left "sed" assigned msys2 "sed" to be in the path, so I never changed that.

https://github.com/AndreMikulec/plr/blob/237812c25c813b3a64b07a1c36fd4fa90bce29ce/appveyor.yml#L461

$env:Path += ";C:\msys64\usr\bin;C:\msys64\mingw64\bin; . . . ETC . . .

I need (or should have) the server_version_number
https://github.com/AndreMikulec/plr/blob/237812c25c813b3a64b07a1c36fd4fa90bce29ce/appveyor.yml#L732

- psql --quiet --tuples-only -c "\pset footer off" -c "\timing off" -c "select current_setting('server_version_num')::integer;"  --output=server_version_num.txt
- bash --login -c "$(cygpath ${APPVEYOR_BUILD_FOLDER})/server_version_num.sh"
- for /f "delims=" %%i in (server_version_num.txt) do (set server_version_num=%%i)
- type  server_version_num.txt
- echo %server_version_num%

I created this .sh file to clean and extract the server_version_num from the psql output:
https://github.com/AndreMikulec/plr/blob/237812c25c813b3a64b07a1c36fd4fa90bce29ce/server_version_num.sh

set -v -x

cd "$(dirname "$0")"

# remove whitespace and empty lines -- except remains: last line that is empty
sed -i -r -e  's/\s+//g' -e '/^$/d' $(cygpath ${APPVEYOR_BUILD_FOLDER})/server_version_num.txt

# remove the only and last empty line
echo -n $(cat $(cygpath ${APPVEYOR_BUILD_FOLDER})/server_version_num.txt) > $(cygpath ${APPVEYOR_BUILD_FOLDER})/server_version_num.txt

set +v +x

Last I patched the file: expected\bad_fun.out..
https://github.com/AndreMikulec/plr/blob/237812c25c813b3a64b07a1c36fd4fa90bce29ce/appveyor.yml#L748

- ps: |
    Set-PSDebug -Trace 2
    if (("$env:pgversion" -eq "master") -and ([double]"$env:server_version_num" -ge 140000.0)) {
      echo "BEGIN change expected\bad_fun.out from ERROR to WARNING"
      echo "PRINTING expected\bad_fun.out"
      type "expected\bad_fun.out"
      # should have returned ERROR - but windows(msvc) returns WARNING(wrong)
      # forcing a regression test to pass
      sed -i "s/ERROR/WARNING/" "expected\bad_fun.out"
      echo "PRINTING expected\bad_fun.out"
      type "expected\bad_fun.out"
      echo "BEGIN change expected\bad_fun.out from ERROR to WARNING"
    }

The line of must importance:
https://github.com/AndreMikulec/plr/blob/237812c25c813b3a64b07a1c36fd4fa90bce29ce/appveyor.yml#L757

sed -i "s/ERROR/WARNING/" "expected\bad_fun.out"

The run result is successful.
https://ci.appveyor.com/project/AndreMikulec/plr/builds/38642994?fullLog=true#L6062

============== creating database "pl_regression"      ==============
CREATE DATABASE
ALTER DATABASE
ALTER DATABASE
ALTER DATABASE
ALTER DATABASE
ALTER DATABASE
ALTER DATABASE
============== running regression test queries        ==============
test plr                          ... ok         1026 ms
test bad_fun                      ... ok          340 ms
test opt_window                   ... ok          266 ms
test do                           ... ok          221 ms
test out_args                     ... ok          259 ms
=====================
 All 5 tests passed. 
=====================

I hope this is helpful.

Thanks,
Andre

from plr.

Dave,

So, now plr-on-Windows, builds without an error at "test bad_fun".

msvc:
https://ci.appveyor.com/project/AndreMikulec/plr/builds/38744261/job/6egjrkhp3sw0rg6x?fullLog=true#L2472

test bad_fun                      ... ok          295 ms

What did one do, to fix the problem?

from plr.

see #91

from plr.

@AndreMikulec I expect this is still a valid issue. Do we have a PR to apply to merge this in ?

from plr.

Dave,

This is a valid issue. I am just saying:

Every time someone shifts a line in master in Mkvcbuild.pm (or vcregress.pl), one has to create a new msvc.diff file.

For example, one is limited, in such as way, that one can not use msvc to compile form source, PostgreSQL 12 (that is currently, still supported). So, if someone ask for a msvc "plr that is compiled from PostgreSQL 12 source", one can not build that plr.

That file Mkvcbuild.pm is under heavy maintenance and is always under risk of line shifts of frequency, once or twice per month. Take a look at the commit history:

https://github.com/postgres/postgres/commits/master/src/tools/msvc/Mkvcbuild.pm

What do you want to do next?

from plr.

I need a PR so that I can merge it in.

Also if you can review #56 that would be awesome

from plr.

Dave,

O.K. I will start working on the PR.
O.K., Just after, I will take a look at #56

from plr.