fix compile with Visual Studio on windows #720

dktapps · 2017-07-11T12:07:15Z

AF_UNIX is defined in WinSock2.h which is included by php_network.h, meaning AF_UNIX is never not defined. However, Windows doesn't support Unix domain sockets.

This fixes #713 .

Jan-E · 2017-07-19T13:01:13Z

@dktapps I applied the patch to the fork by @sirsnyder
sirsnyder#5

The test results aren't bad:

=====================================================================
Number of tests :   99                98
Tests skipped   :    1 (  1.0%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    3 (  3.0%) (  3.1%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :   95 ( 96.0%) ( 96.9%)
---------------------------------------------------------------------
Time taken      :    3 seconds
=====================================================================
=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
Basic anonymous class support, fix #505 [C:\projects\php-src\ext\pthreads\tests\inherited-anon-class-outside-context.phpt]
Test fix for #658 with inheritance [C:\projects\php-src\ext\pthreads\tests\inherited-anon-class.phpt]
Test fix for #659 [C:\projects\php-src\ext\pthreads\tests\no-inheritance-anon-class.phpt]
=====================================================================

See https://ci.appveyor.com/project/SirSnyder/pthreads-ftdtq/build/feature/winbuild.13 (at the bottom)

Jan-E · 2017-07-19T13:18:41Z

Try changing this line in appveyor.yml

git clone https://github.com/php/php-src C:\projects\php-src

to

git clone -b PHP-7.0 https://github.com/php/php-src C:\projects\php-src

to see what Appveyor makes of your patch to @krakjoe 's version.

dktapps · 2017-07-19T14:14:56Z

I attempted to set-up an environment matrix for AppVeyor on my own fork to have it build for 7.0 and 7.1 both, but it was too much of a pain.

dktapps · 2017-07-19T14:18:06Z

Ah. AppVeyor won't build this PR anyway, the appveyor.yml is configured to build master only.

Jan-E · 2017-07-19T14:30:48Z

No, pthreads master is now restricted to build with PHP 7.2 only:
https://github.com/krakjoe/pthreads/blob/master/php_pthreads.c#L38

pthreads requires PHP 7.2, ZTS in versions 7.0 and 7.1 is broken

See 5bb2058
Maybe @krakjoe van revert that when he sees https://ci.appveyor.com/project/SirSnyder/pthreads-ftdtq/build/feature/winbuild.13

Jan-E · 2017-07-19T14:39:17Z

You removed your commit, but the Appveyor output is still there ;-)
https://ci.appveyor.com/project/krakjoe/pthreads/build/master.161#L999

dktapps · 2017-07-19T14:42:02Z

My branch doesn't have the commit which prevents building on < 7.2... not sure what's going on there.

I don't have VS2017 on my machine at the moment because I still need VS2015 for various things, so I can't test this against 7.2 immediately. I'll have to see if I can get them to coexist.

Jan-E · 2017-07-19T14:52:41Z

My branch doesn't have the commit which prevents building on < 7.2... not sure what's going on there.

Your patch is againts @krakjoe's version, which has the commit..

There is no problem in having VS2017 besides VS2015, with one caveat:
https://www.apachelounge.com/viewtopic.php?p=35244

dktapps · 2017-07-20T20:21:32Z

My last commit fixes AppVeyor to build against 7.2dev, but don't be fooled by the test result - there's something wrong with pthreads on 7.2 with Windows (the exact same build process worked perfectly with 7.1 and all tests succeeded (without the last 5 commits to master or so)) - lots of the tests fail for reasons I can't explain.

Jan-E · 2017-07-21T02:48:18Z

See https://www.apachelounge.com/posting.php?mode=editpost&p=30903 for my builds
With https://phpdev.toolsforresearch.com/php-7.2.0beta1-Win32-VC15-x86.zip test results are almost perfect.

php /php-sdk/php72dev/run-tests.php /php-sdk/php72dev/ext/pthreads/tests/*.phpt

=====================================================================
Number of tests :   97                96
Tests skipped   :    1 (  1.0%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    1 (  1.0%) (  1.0%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :   95 ( 97.9%) ( 99.0%)
---------------------------------------------------------------------
Time taken      :   10 seconds
=====================================================================

=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
Test Volatile objects [/php-sdk/php72dev/ext/pthreads/tests/volatile-objects.phpt]
=====================================================================

php.ini:
[PHP]
extension_dir = "ext"
extension=php_pthreads.dll
`
TEST_PHP_EXECUTABLE:
/php-sdk/php72dev/Release_TS/php-7.2.0beta1/php.exe

Jan-E · 2017-07-21T04:38:24Z

It must be something in the test setup. I downloaded your master artificial and ran the tests with a php.ini:

=====================================================================
Number of tests :   97                96
Tests skipped   :    1 (  1.0%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    0 (  0.0%) (  0.0%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :   96 ( 99.0%) (100.0%)
---------------------------------------------------------------------
Time taken      :    4 seconds
=====================================================================

C:\phpdev\master>type php.ini
[PHP]
extension_dir = "ext"
extension=php_pthreads

Wes0617 · 2017-07-21T07:34:11Z

since php 7.2 the only supported compiler will be vs2017 (VC15), not vs2015 (VC14)

in case you don't know, and unsure if relevant information...

dktapps · 2017-07-21T10:10:14Z

@Jan-E if you run the tests with --show-diff, why are they failing for you? When I test locally, random tests are failing because pthreads is apparently already loaded. It's never the same tests, always random ones. Worked fine on 7.1.

Jan-E · 2017-07-21T12:12:42Z

@dktapps With the php_pthreads.dll from the Appveyor build I have no failing tests. With my build, this resulted in a Unhandled exception: read access violation aka a crash:

php.exe \php-sdk\php72dev\ext\pthreads\tests\volatile-objects.php

The crash is not detected by run-tests.php, but volatile-objects.out is just 0 bytes. I am trying to find out what is the difference between your Appveyor build and mine. Maybe it is my custom pthreadsVC2.lib/dll (with a pthreadsVC2.pdb, maybe it is because you are building PHP 7.2.0-dev and I build PHP 7.2.0-beta1.

When I test locally, random tests are failing because pthreads is apparently already loaded. It's never the same tests, always random ones.

Strange. See my tests with the master artefact that I downloaded from the Appveyor build. Did you use that one as well?

Jan-E · 2017-07-21T12:44:23Z

Downloaded http://windows.php.net/downloads/qa/php-7.2.0beta1-Win32-VC15-x86.zip added php_pthreads.dll and pthreadVC2.dll from the master artefact, added a php.ini and a tests.bat. Result:

=====================================================================
Number of tests :   97                96
Tests skipped   :    1 (  1.0%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    0 (  0.0%) (  0.0%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :   96 ( 99.0%) (100.0%)
---------------------------------------------------------------------
Time taken      :    5 seconds
=====================================================================

C:\phpdev\php-7.2.0beta1-Win32-VC15-x86>type tests.bat
\phpdev\php-7.2.0beta1-Win32-VC15-x86\php.exe -doutput_buffering=0 /php-sdk/php72dev/run-tests.php /php-sdk/php72dev/ext/pthreads -p \phpdev\php-7.2.0beta1-Win32-VC15-x86\php.exe -q --show-diff

C:\phpdev\php-7.2.0beta1-Win32-VC15-x86>type php.ini
[PHP]
extension_dir = "."
extension=php_pthreads

C:\phpdev\php-7.2.0beta1-Win32-VC15-x86>dir /o:-d
 Volume in drive C is Windows7_OS
 Volume Serial Number is D13A-4B50

 Directory of C:\phpdev\php-7.2.0beta1-Win32-VC15-x86

21/07/17  14:23                49 php.ini
21/07/17  14:23               196 tests.bat
21/07/17  14:22    <DIR>          ..
21/07/17  14:22    <DIR>          .
21/07/17  14:19    <DIR>          sasl2
21/07/17  14:19    <DIR>          lib
21/07/17  14:19    <DIR>          extras
21/07/17  14:19    <DIR>          ext
21/07/17  14:19    <DIR>          dev
20/07/17  13:19            75.776 php_pthreads.dll
20/07/17  13:19            55.808 pthreadVC2.dll
19/07/17  01:47            53.242 pharcommand.phar
19/07/17  01:47                43 phar.phar.bat
19/07/17  01:42            15.360 gmodule-2.dll
19/07/17  01:42            79.396 install.txt
19/07/17  01:42            41.984 icuio57.dll
19/07/17  01:42         2.143.744 libcrypto-1_1.dll
19/07/17  01:42            35.328 libenchant.dll
19/07/17  01:42           151.040 libpq.dll
19/07/17  01:42         1.233.920 icuuc57.dll
19/07/17  01:42         1.814.016 icuin57.dll
19/07/17  01:42           183.808 libssh2.dll
19/07/17  01:42           380.928 libssl-1_1.dll
19/07/17  01:42        25.671.680 icudt57.dll
19/07/17  01:42            16.830 news.txt
19/07/17  01:42           156.672 nghttp2.dll
19/07/17  01:42            44.544 iculx57.dll
19/07/17  01:42            82.432 libsasl.dll
19/07/17  01:42            13.556 snapshot.txt
19/07/17  01:42           229.376 icule57.dll
19/07/17  01:42            20.183 readme-redist-bins.txt
19/07/17  01:42             2.523 php.gif
19/07/17  01:42            97.280 deplister.exe
19/07/17  01:42            72.305 php.ini-development
19/07/17  01:42            72.584 php.ini-production
19/07/17  01:42         1.194.496 glib-2.dll
19/07/17  01:42           488.448 libsodium.dll
19/07/17  01:42             3.286 license.txt
19/07/17  01:42         7.167.488 php7ts.dll
19/07/17  01:42           276.480 phpdbg.exe
19/07/17  01:42           852.930 php7embed.lib
19/07/17  01:42            30.720 php7apache2_4.dll
19/07/17  01:42           105.984 php.exe
19/07/17  01:42            33.280 php-win.exe
19/07/17  01:42            57.344 php-cgi.exe
19/07/17  01:42           225.792 php7phpdbg.dll
              39 File(s)     43.180.851 bytes
               7 Dir(s)  62.041.874.432 bytes free

C:\phpdev\php-7.2.0beta1-Win32-VC15-x86>

dktapps · 2017-07-21T15:59:28Z

Yep, doesn't matter what version I use, I see this at random intervals:

PHP Warning:  Module 'pthreads' already loaded in Unknown on line 0

Warning: Module 'pthreads' already loaded in Unknown on line 0

AppVeyor is being even more strange by frequently declaring that random pthreads classes don't exist. I don't know what's going on there.

Jan-E · 2017-07-21T16:53:54Z

I do not know either what is going wrong. I rebuilt my ptreadsVC2.dll/lib, cloned this repo master, applied your patch and now got this result:

=====================================================================
Number of tests :   97                96
Tests skipped   :    1 (  1.0%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    0 (  0.0%) (  0.0%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :   96 ( 99.0%) (100.0%)
---------------------------------------------------------------------
Time taken      :    6 seconds
=====================================================================
cmdline:
php -doutput_buffering=0 /php-sdk/php72dev/run-tests.php /php-sdk/php72dev/ext/pthreads -q --show-diff
php.ini:
[PHP]
extension_dir = "ext"
extension=php_pthreads
TEST_PHP_EXECUTABLE:
/php-sdk/php72dev/Release_TS/php-7.2.0beta1/php.exe

I am now building the x64 version.

Jan-E · 2017-07-21T17:34:32Z

x64 build results:

=====================================================================
Number of tests :   97                96
Tests skipped   :    1 (  1.0%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    0 (  0.0%) (  0.0%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :   96 ( 99.0%) (100.0%)
---------------------------------------------------------------------
Time taken      :    4 seconds
=====================================================================
cmdline:
php -doutput_buffering=0 /php-sdk/php72dev/run-tests.php /php-sdk/php72dev/ext/pthreads -q --show-diff
php.ini:
[PHP]
extension_dir = "ext"
extension=php_pthreads
TEST_PHP_EXECUTABLE:
/php-sdk/php72dev/x64/Release_TS/php-7.2.0beta1/php.exe

Jan-E · 2017-07-21T17:43:01Z

Yep, doesn't matter what version I use, I see this at random intervals:

Try setting TEST_PHP_EXECUTABLE before running run-tests.php, do not use the -p.
cd to the directory of the php.exe and add a minimal php.ini there.
This is how I run my tests. See output above.

Jan-E · 2017-07-21T17:51:53Z

@krakjoe 67bd55f is OK to merge

The Appveyor setup is much better than what you have right now, so it might be OK to merge that as well.

Jan-E · 2017-07-21T17:54:09Z

@dktapps Maybe change the title and add VS2017.

Jan-E · 2017-07-22T11:03:06Z

Thanks. Maybe I will try to fix the test setup now.

Jan-E · 2017-07-22T11:17:31Z

@krakjoe If I do a git checkout 5272863 and apply @dktapps 's patch, below is the effect on PHP 7.1,
"pthreads requires PHP 7.2, ZTS in versions 7.0 and 7.1 is broken" does not seem to be true.

pthreads/php_pthreads.c

Line 38 in 5bb2058

# error "pthreads requires PHP 7.2, ZTS in versions 7.0 and 7.1 is broken"

Time to revert (parts of) 5bb2058 ???

=====================================================================
Number of tests :   97                96
Tests skipped   :    1 (  1.0%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    0 (  0.0%) (  0.0%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :   96 ( 99.0%) (100.0%)
---------------------------------------------------------------------
Time taken      :    5 seconds
=====================================================================
cd:
N:\php-sdk\php71dev\Release_TS\php-7.1.8RC1
cmdline:
php -doutput_buffering=0 /php-sdk/php71dev/run-tests.php /php-sdk/php71dev/ext/pthreads -q --show-diff
php.ini:
[PHP]
extension_dir = "ext"
extension=php_pthreads.dll
TEST_PHP_EXECUTABLE:
N:\php-sdk\php71dev\Release_TS\php-7.1.8RC1\php.exe
PHP 7.1.8RC1 (cli) (built: Jul 22 2017 11:48:41) ( ZTS MSVC14 (Visual C++ 2015) x86 )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2017 Zend Technologies

dktapps · 2017-07-22T11:24:12Z

The reason for cutting < 7.2 support was discussed here and here. Regardless of the test results, I can confirm that since my project moved to PHP 7.0 we have been experiencing a lot of seemingly random faults which do not occur anymore with PHP 7.2.

Jan-E · 2017-07-22T11:40:59Z

Wow. Am I reading this right? Is ZTS in PHP 7.0 and 7.1 broken, regardless of pthreads? That would justify an ABI break in the ZTS builds of 7.0 and 7.1...

Jan-E · 2017-07-23T02:22:18Z

Try setting TEST_PHP_EXECUTABLE before running run-tests.php, do not use the -p.
cd to the directory of the php.exe and add a minimal php.ini there.

#722 I'm feeling lucky. Direct hit:
https://ci.appveyor.com/project/krakjoe/pthreads/build/master.167#L645

dktapps added 2 commits July 20, 2017 21:14

Compiles, don't know if all socket stuff works though.

67bd55f

AppVeyor builds 7.2, but something is very wrong

f8ba90f

dktapps changed the title ~~fix compile with VS2015 on windows~~ fix compile with Visual Studio on windows Jul 21, 2017

krakjoe merged commit 14a8236 into krakjoe:master Jul 22, 2017

dktapps deleted the windows-compile branch July 22, 2017 11:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix compile with Visual Studio on windows #720

fix compile with Visual Studio on windows #720

dktapps commented Jul 11, 2017

Jan-E commented Jul 19, 2017 •

edited

Jan-E commented Jul 19, 2017

dktapps commented Jul 19, 2017

dktapps commented Jul 19, 2017 •

edited

Jan-E commented Jul 19, 2017

Jan-E commented Jul 19, 2017

dktapps commented Jul 19, 2017 •

edited

Jan-E commented Jul 19, 2017 •

edited

dktapps commented Jul 20, 2017 •

edited

Jan-E commented Jul 21, 2017

Jan-E commented Jul 21, 2017

Wes0617 commented Jul 21, 2017

dktapps commented Jul 21, 2017

Jan-E commented Jul 21, 2017

Jan-E commented Jul 21, 2017

dktapps commented Jul 21, 2017

Jan-E commented Jul 21, 2017 •

edited

Jan-E commented Jul 21, 2017

Jan-E commented Jul 21, 2017

Jan-E commented Jul 21, 2017

Jan-E commented Jul 21, 2017

Jan-E commented Jul 22, 2017

Jan-E commented Jul 22, 2017

dktapps commented Jul 22, 2017

Jan-E commented Jul 22, 2017

Jan-E commented Jul 23, 2017

fix compile with Visual Studio on windows #720

fix compile with Visual Studio on windows #720

Conversation

dktapps commented Jul 11, 2017

Jan-E commented Jul 19, 2017 • edited

Jan-E commented Jul 19, 2017

dktapps commented Jul 19, 2017

dktapps commented Jul 19, 2017 • edited

Jan-E commented Jul 19, 2017

Jan-E commented Jul 19, 2017

dktapps commented Jul 19, 2017 • edited

Jan-E commented Jul 19, 2017 • edited

dktapps commented Jul 20, 2017 • edited

Jan-E commented Jul 21, 2017

Jan-E commented Jul 21, 2017

Wes0617 commented Jul 21, 2017

dktapps commented Jul 21, 2017

Jan-E commented Jul 21, 2017

Jan-E commented Jul 21, 2017

dktapps commented Jul 21, 2017

Jan-E commented Jul 21, 2017 • edited

Jan-E commented Jul 21, 2017

Jan-E commented Jul 21, 2017

Jan-E commented Jul 21, 2017

Jan-E commented Jul 21, 2017

Jan-E commented Jul 22, 2017

Jan-E commented Jul 22, 2017

dktapps commented Jul 22, 2017

Jan-E commented Jul 22, 2017

Jan-E commented Jul 23, 2017

Jan-E commented Jul 19, 2017 •

edited

dktapps commented Jul 19, 2017 •

edited

dktapps commented Jul 19, 2017 •

edited

Jan-E commented Jul 19, 2017 •

edited

dktapps commented Jul 20, 2017 •

edited

Jan-E commented Jul 21, 2017 •

edited