On 30 November 2016 at 23:17, Olly Betts <***@survex.com> wrote:
On Wed, Nov 30, 2016 at 09:49:49PM +0000, Olly Betts wrote:
OK - I missed this because if you had both installed, then the PHP5 examples
Post by Olly Bettswere just being generate, built and run using PHP7, which works because
none of
the differences between the two matter at this level. Now fixed.
Looking better on my local box now.
Post by Olly BettsAdding checks for this, though they wouldn't have fired in the above case
-
Post by Olly Bettseither the PHP7 stuff isn't installed and you get the error you did, or
else
Post by Olly Bettsit is and everything consistently used PHP7.
Yes, I see you've put this in. I was just thinking it'll generally be a
little more user friendly as the world transitions from php5 to php7.
Post by Olly BettsPost by William S FultonHave you tested this with both php5 and php7 installed and on a box and
with just one of the two versions installed? Probably this needs doing and
it will flush out the problems.
I've now tested with just php5 installed and with both installed but all
the
Post by Olly Bettsphp5 binaries and header files moved aside, and the testsuite passes for
the installed version and skips for the other in each case, as it ought
to.
Post by Olly BettsI did "make distclean" in between tests.
I've also successfully built working PHP7 bindings for Xapian using this
new code.
Yes, congratulations on great work to all involved, this is a great step
forward.
Post by Olly BettsPost by William S FultonDoes 'swig -php' result in php5 or
php7 wrappers? I trust this change on a stable maintenance branch it
means
Post by William S Fultonexactly the same as swig-3.0.10.
PHP5, as before (and as discussed in the ticket).
Understood now, thanks.
Why was this commit made without any Travis tests being added? The Travis
Post by William S Fultontests show it is using php 5.5.9, however, this is with the languages as
'php'. What exactly does 'php' mean now? On my box which has php 5.6
'php'
Post by William S Fultonis skipped, but php5 nearly works as shown above.
I attempted to ensure the new backend fulfilled all the documented
prerequisites before merging it. Those prerequisites don't include CI
http://www.swig.org/Doc3.0/Extending.html#Extending_prerequisites
In fact I more than fulfilled some of them - the *ENTIRE* testsuite passes,
not just the majority.
Yes, having extensive test coverage is ideal. I'm sure we can get this
working on Travis soon enough. All the extensive effort put into the
testing will be wasted if they aren't run in a CI environment.
Post by Olly BettsFrom what I can make out there is something completely out of step though.
make-xx-test-suite and make-xx-examples etc should invoke swig -xx, where
xx is a target language. This is how the Makefiles and all the testing
works. From what I can see make-php-examples does not invoke swig -php, it
invokes swig -php7 when it should invoke swig -php (php5), so actually
invokes the wrong version of php. I don't want this inconsistency and I
believe is why Travis is constantly failing now. Can you fix it without
renaming lots of directories and file names? If not, let's git reset --hard
and go back to an earlier revision and fix it up so the history on master
isn't a mess.
I didn't follow the letter of #7, in that my patch was provided via
Post by Olly Bettsgithub rather than email, but I feel that's actually more useful with
our current infrastructure - it's easier to browse the changes (and
you can get a flat file patch easily) plus mailing lists tend to eat large
attachments.
That page needs updating, I'll have a go at it. I'm sure you've noticed
the precedent over the last few years since we moved to using Github +
Travis of nearly always insisting on a test that can run on Travis before
accepting code changes. Adding a whole module clearly should be no
different and in my view a non-negotiable prerequisite
Post by Olly BettsAs for adding CI, please see my comment in the ticket, where I asked you how to
add it. There aren't PHP7 packages in trusty, and I'm no expert in travis so I
don't know if we can just ask for a xenial VM/container/chroot/whatever, or how
best to pull packages from a PPA if we have to use trusty.
Just seen the comment. A bit more time to reply and an email warning to
swig-devel before the merge into master would have been appreciated.
Travis supports docker images. Anyone here familiar with Docker? In theory
it should be a simple job for someone familiar with this to set up a docker
image with OS of choice as the SWIG dependencies are so simple. Otherwise
it looks really simple to upgrade to php7 on Trusty (which is available on
Travis), see
https://launchpad.net/~ondrej/+archive/ubuntu/php?field.series_filter=trusty
and
https://www.digitalocean.com/community/tutorials/how-to-upgrade-to-php-7-on-ubuntu-14-04.
Anyone can test Travis using a personal account and I suggest doing this as
the .travis file can be whittled down to just php in any personal branch
with a quick turn-around.
Post by Olly BettsHave you tested this with both php5 and php7 installed and on a box and
Post by William S Fultonwith just one of the two versions installed? Probably this needs doing
and
Post by William S Fultonit will flush out the problems.
I've tested with both installed. Uninstalling PHP5 is troublesome, as those
packages are gone from Debian testing now so it's a pain to reinstall them, and
I really need to have them installed or else I can't usefully maintain SWIG's
PHP5 support (and nobody else seems interested). I guess I can rename the
header directory temporarily.
I've got php5.6 on Xenial, but Xenial does come with php7 too. I will try
install both on a VM and see how that goes. If you havn't got familiar with
virtual machines yet, I highly recommend it. It is exactly case like this
where one can do some risky/awkward testing and mix old and new versions
and easily roll back the changes without upsetting your main computer. Kind
of essential for the myriad of versions of everything we need to support in
SWIG.
William