Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PHP Mapscript via SWIG #5642

Closed

Conversation

AlexanderGabriel
Copy link
Contributor

Hi,

another try.
This will add support for PHP Mapscript via SWIG and it builds with travis and appveyor.

Kind Regards,
Alex

@geographika
Copy link
Member

I see you need to build SWIG from source to get this to work on Windows. Any idea when a SWIG release is planned that will produce the PHP MapScript?

It may also be worth moving the PHP build commands to a separate script file in appveyor.yaml in the future (I'll try to do the same with Python) to keep it easier to read (similar to the Travis script).

@AlexanderGabriel
Copy link
Contributor Author

It' even worse. I build a developement state of swig (didnt update for a while) for PHP but we still need swig from gisinternals sdk for csharp... it is not possible to use the built swig for csharp because of any other bug.
this is of course only a workaround. when a version of swig will be released that does not have any problems with php and csharp i don't know.

building php would be unnessesary aswell if it was in the sdk or the needed files were available as download somewhere.

I really tried a lot of things. look at the commit history of the other PR i already closed and this is only the stuff i pushed to github.
I didn't want to upload php or swig compiled to my homepage and create another unrelyable dependency.
This was the way/workaround I was able to get this working and i think, this is a good point to start and improve this.
If you seperate the python commands, I can look how that works and do this with php, too but I'm really a beginner with appveyor ;)

@geographika
Copy link
Member

I tried to get the csharp bindings working with a later version of SWIG (as the Python bindings should really be built with Swig > 3.0) - see #5583
I'm not a csharp developer, but will take another look at this soon.

In case you are wondering why all the slashes are reversed in the appveyor.yaml paths - CMake throws errors on paths with \t and other command characters.

We could maybe use Chocolatey for installation of various dependencies (SWIG, PHP etc), see
https://chocolatey.org/packages/php

The existing pull request looks good to me though and can be refined in the future.

@AlexanderGabriel
Copy link
Contributor Author

AlexanderGabriel commented Aug 16, 2018

I tried chocolatey and installed php but it does not contain dvelopement files
A few years ago, php offered all that to download but they do not any more.

@AlexanderGabriel
Copy link
Contributor Author

Would it be better if i opened the pullrequest against master or can you switch this when merging?

@geographika
Copy link
Member

@AlexanderGabriel - would you be able to open against the master branch?
Also rather than building SWIG from source how about downloading form http://prdownloads.sourceforge.net/swig/swigwin-3.0.12.zip - this can then be cached in Appveyor which will speed up build times.
Are all PHP versions required for Travis? The build time is up to 51 minutes (see https://travis-ci.org/mapserver/mapserver/builds/416091387). Perhaps one of the 5.x versions could be dropped.

I'll then test locally and see if we can get this merged.

@geographika geographika mentioned this pull request Sep 28, 2018
@AlexanderGabriel
Copy link
Contributor Author

Of course. Working on it right now. I dropped building with PHP 5.5. It isn's supported any more so we don't have to test with it.
I need SWIG from git because it fixes a problem when building thread-safe on Windows but PHP and SWIG could be cached. This would save time.

@geographika
Copy link
Member

@AlexanderGabriel - excellent!
I have finally got the .NET MapScript bindings working with SWIG 3.0.12 - see #5583
This includes a few more Appveyor changes but it should be easy enough to merge later.
Does SWIG 3.0.12 resolve the thread-safe issue? It means that PHP and .NET bindings could be combined in a single build step. I also plan on adding Python3 to Appveyor - this could be combined with one of the PHP versions to speed up build time.

@AlexanderGabriel
Copy link
Contributor Author

@geographika SWIG 3.0.12 does not solve this problem. it is from january and the fix is from december last year: swig/swig@224bb9e
In addition, there is no build system provided for windows. i created cmake files to be able to build it. not really a good solution but i didn't find any other way to get this running.
building php while building mapserver is not optimal at all but i couldnt find the needed files on php homepage to download... i could stage the stuff on my website. this would speed up everything but would only add more unnessesary dependencies to external ressources.
I'll prepare a pull request now against master branch and then we can try to improve this over time.
I'll be back ;)

@geographika
Copy link
Member

@AlexanderGabriel - it is possible in Appveyor to leave files/folders between builds which could be done for both SWIG and PHP. Personally I wouldn't be against hosting the compiled files/libs externally and downloading them similar to the SDK. Ideally the PHP libs could be added to the SDK at GISInternals. This would retain the focus of Appveyor on the MapServer build and CI. Maybe it is something to take to the mailing list?

@AlexanderGabriel
Copy link
Contributor Author

@geographika I already cache the sdk in my working branch (https://github.com/AlexanderGabriel/mapserver/tree/TryToFixAppVeyorAndSpeedup), now working on php. it costs me so much time to wait every time so i think, it's worth it.
the sdk would really be a good place to keep php and swig (in two versions until new swig version gets released). in the readme under mapscript/phpng is everything that is needed to build php and swig.

@geographika
Copy link
Member

I've had a quick attempt to run things locally. A couple of questions.

SWIG compiled fine. I presume the patch required for PHP will be in the future SWIG 4.0 release?
I guess this could happen in the near future: swig/swig#1299

With regards to the compiling PHP, is it possible to use the binaries at https://windows.php.net/download#php-7.2-ts-VC15-x64 ? Or does this output differ from the Appveyor steps?

@AlexanderGabriel
Copy link
Contributor Author

Hi Seth,

i thought about this today, too. I need current swig "only" for thread-safe php&mapscript on windows. Non-thread-safe mapscript for windows is really unuseful (can't load php it into apache as module for example) but we will for sure not be happy if mapserver relies on unstable software-dependencies.
In addition, appveyor changed cmake to 3.12 last week and now the building fails because swig cannot find swig.swg but only if i use my build so there is something broken aswell or this is a bug in cmake on windows. did you have any problems with SWIG in AppVeyor since 28th of Sept?
so i thought it would be best to go one step back, build php-mapscript non-thread-safe but based on stable releases like swig 3.0.12 (which works) and list thread-safety-support as known issue until swig-4.0.0 is released. this will be much less work for all of us.

for the cache: php is too big (>1GB) but i hat the idea to delete dependencies after compiling, zipping everything and download it from my server or cache it. depends on what is faster. even don't do a snapshot build saves time.

So what i will do now is:

  • Build everything for Non-Thread-Safe PHP
  • Use stable SWIG 3.0.12
  • Make it work with CMake 3.12.x

Would that be ok?
And after SWIG-4.0 gets released, we solve the thread-safe-problem on windows. I think, theres more people waiting for php-mapscript-support on linux than on windows ;)

@AlexanderGabriel
Copy link
Contributor Author

With regards to the compiling PHP, is it possible to use the binaries at https://windows.php.net/download#php-7.2-ts-VC15-x64 ? Or does this output differ from the Appveyor steps?

No. This does not contain development header... I tried this in the beginning. A few years ago, php offered headers to download...

@AlexanderGabriel
Copy link
Contributor Author

this can last a few days more. not sure, what the problem is. i opened an issue at cmake. can't build phpng-mapscript with cmake 3.12.3 but 3.11.4 works: https://gitlab.kitware.com/cmake/cmake/issues/18421

@geographika
Copy link
Member

@AlexanderGabriel - I've sent a mail to the dev list to see if hosting the PHP SDK and downloading to Appveyor is ok. See https://lists.osgeo.org/pipermail/mapserver-dev/2018-October/015511.html
Is it ok with the hosting costs on your side?
Agree - let's go with SWIG 3.0.12 - I have this setup and ready to add in #5583 - and will merge to master this weekend if there are no objections.

Do we need to test all PHP v7 versions on Travis including nightly? Would sticking to just 7.2 be enough, and document this is the "supported/tested" version?

I've read through the details in #5557 and the RFC on the approach taken, and have a couple of questions.

Is there any easy test test that can be added to verify the build (even just outputting the MapServer version number through PHPNG MapScript)? The php7module.i doesn't contain any custom code - is this because it is unnecessary, or is this something to be added to in the future?

We'll get this merged soon!

@AlexanderGabriel
Copy link
Contributor Author

Agree - let's go with SWIG 3.0.12 - I have this setup and ready to add in #5583 - and will merge to master this weekend if there are no objections.

Already saw this. Will wait for the merge.
I was able to strip down PHP-SDK to 168MB after building and mapserver still builds. Disabling different modules saves time, aswell. We'll see if it is necessary to buffer this somewhere or build it every time. Hosting it on my server is ok.

Do we need to test all PHP v7 versions on Travis including nightly? Would sticking to just 7.2 be enough, and document this is the "supported/tested" version?

We can drop this. Travis is new to me so i was playing around with it. Thought it would be cool to build against nightly builds ;)

I've read through the details in #5557 and the RFC on the approach taken, and have a couple of questions.

Not sure if i understand this. You have questions to me? Shall we meet in the irc?

Is there any easy test test that can be added to verify the build (even just outputting the MapServer version number through PHPNG MapScript)?

Sure. phpinfo() or something and analyze the output. Didn't try something like this yet.

The php7module.i doesn't contain any custom code - is this because it is unnecessary, or is this something to be added to in the future?

Not sure, why there should be custom code. The interface is defined in mapscript.i. If everything is fine, there must not be any code here? This file exists because @geomunion added some information to phpinfo().
There is one message when runnig swig:
CustomBuild:
Swig source
C:\projects\mapserver\mapscript\v8....\mapserver.h(932): warning 314: 'empty' is a PHP keyword, renaming to 'c_empty' [C:\projects\mapserver\build\mapscript\phpng\php_mapscriptng.vcxproj]
I think, this can be masked in php7module.i but there is nothing more i plan to put there but hey - we are at the beginning ;)

@AlexanderGabriel
Copy link
Contributor Author

@geographika
i tried this in CMakeLists.txt of phpng-mapscript on my machine and it works fine:
file(DOWNLOAD "https://files.digital-infinity.de/php-sdk-$ENV{VSCMD_ARG_TGT_ARCH}-nts.tar.gz" "${PROJECT_SOURCE_DIR}/../php-sdk.tar.gz")
execute_process(COMMAND ${CMAKE_COMMAND} -E tar xzf "${PROJECT_SOURCE_DIR}/../php-sdk.tar.gz" WORKING_DIRECTORY "${PROJECT_SOURCE_DIR}/..")

Letting CMake download all that stuff would not only clean up the appveyor.yml. It would work even for users when building mapserver. they will not have any work with downloading dependencies... everything is downloaded just in time. even swig-3.0.12 in you csharp module.

tar is built in but you can call 7z aswell but users have to install it. repacking mapserver-sdk to tar.gz would solve this.

@AlexanderGabriel
Copy link
Contributor Author

sorry. wrong button...

@geomunion
Copy link
Contributor

The php7module.i is used to generate php-module specific code to display MapScript version etc. via

<?php
phpinfo();

see:

@geographika
Copy link
Member

@AlexanderGabriel - you answered my questions in your above comment - thanks!
Is it possible to download https://files.digital-infinity.de/php-sdk-$ENV{VSCMD_ARG_TGT_ARCH}-nts.tar.gz as part of the Appveyor script and simply unzip and point paths to the SDK?
It might be better in the long term to keep CMake focused on MapServer and let the CI take care of gathering dependencies - this would also mean users can build with the own dependencies if needed.

@AlexanderGabriel
Copy link
Contributor Author

I think, we can forget php-mapscriptng on windows for now. At least building it with AppVeyor will fail if we don't change CMake during the build.
The current development state of CMake can build the extension again but this is the same problem as with SWIG: It is not stable.

https://gitlab.kitware.com/cmake/cmake/merge_requests/2448#note_466298
CMake 3.13 will be soon so i think, we have to wait.

I can prepare CMakeLists.txt so CMake will throw errors on Windows until the right versions of SWIG and CMake are used. This will at least enable users to compile it if they get the latest development states of SWIG and CMake.

Until that, we will also not need PHP-SDK somewhere. Hosting it on osgeo.org as suggested by Steve is good. We can change the urls as soon as we got the final urls for the sdk.
Should i prepare the sdk with a specific version of php like latest release of 7.2? (7.2.10)

@AlexanderGabriel
Copy link
Contributor Author

Is there any easy test test that can be added to verify the build (even just outputting the MapServer version number through PHPNG MapScript)?

AppVeyor has chocolatey installed. i can install php with cocholatey and it should be easy to install mapscript-module after building and produce some test-output.

@AlexanderGabriel
Copy link
Contributor Author

New PR here: #5675

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants