? ? Success

User tests: Successful: Unsuccessful:

avatar Hackwar
Hackwar
7 Oct 2016

According to the documentation, a DSN connection string should use semicolons as delimiter and not spaces. It should also not make any difference for the normal PHP implementation. Since we have currently ~80 errors in the test suite when running on HHVM and most of these are related to this DSN string, I'm trying to fix this. Funnily, I don't have access to HHVM, so I'm going to try to fix this by running this through the unittests again and again... ? ?

avatar Hackwar Hackwar - open - 7 Oct 2016
avatar Hackwar Hackwar - change - 7 Oct 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 7 Oct 2016
Labels Added: ? ?
avatar joomla-cms-bot joomla-cms-bot - change - 7 Oct 2016
Category Unit Tests
avatar Hackwar
Hackwar - comment - 7 Oct 2016

Seems like this indeed "fixes" that error, however throwing a fatal error because of this: facebook/hhvm#7320

Seems as if we would only support the very latest version of HHVM with Postgres. Can we have such a requirement or do we have to introduce a conditional for that function call?

avatar Hackwar
Hackwar - comment - 7 Oct 2016

So... with this PR, the errors would be reduced from 80 to 11. Is it worth that?

avatar zero-24 zero-24 - change - 7 Oct 2016
Milestone Added:
avatar wilsonge
wilsonge - comment - 7 Oct 2016

Yes it's worth

avatar andrepereiradasilva
andrepereiradasilva - comment - 7 Oct 2016

can't this go for staging and 3.7.x too?

avatar mbabker
mbabker - comment - 7 Oct 2016

Well, I tried changing the base branch but the PR would've drug in the entirety of the 4.0 branch. But yes, this should go into staging instead of 4.0 only.

avatar zero-24
zero-24 - comment - 7 Oct 2016

We can merge it against 4.0-dev and we can open a new PR against staging ?

Done: #12341

avatar photodude
photodude - comment - 7 Oct 2016

These are exactly the changes I have made in my test branches pending release of hhvm 3.15.2 or 3.16 which will resolve these issues. facebook/hhvm#7399 (comment)

avatar photodude
photodude - comment - 7 Oct 2016

BTW some of these issues were cataloged a while back in #10220 although the connection issue was new in 3.15 with the inclusion of pgsl.so in HHVM as noted in facebook/hhvm#7399 (comment)

avatar photodude
photodude - comment - 7 Oct 2016

@Hackwar According to the documentation spaces or semi-colons should work,

The PDO_PGSQL Data Source Name (DSN) is composed of the following elements, delimited by spaces or semicolons:

Fortunately HHVM is working on a fix. But I do agree this is a valid fix/work around (personally, I think in most cases spaces are the worst choice for delimiters)

I have been hesitant to submit the change for our call to pg_set_error_verbosity() since I'm unsure of any possible downstream consequences (I assume possibly none, but you know what they say about assuming). I'm also hesitant since this issue is fixed in hhvm 3.15.2. At the same time, at one point I suggested the same change as a possible work around.

looks good on code review.

avatar Hackwar
Hackwar - comment - 11 Oct 2016

So with those comments, we could merge this. ?

avatar photodude
photodude - comment - 16 Oct 2016

The change to our call of pg_set_error_verbosity() is no longer needed as hhvm 3.15.2 has been released. We do still need the change to the separator for the DSN used in the tests as that patch has not been merged (they requested some additional changes).

avatar photodude
photodude - comment - 17 Oct 2016

The HHVM patch to fix the DSN separators to allow semicolon or space has been merged, the HHVM 3.16.0 STS release will have this, and is on the list of potential cherry picks for HHVM 3.15.3 LTS.

HHVM 3.16.0 STS is expected around 10/24/2016 and will automatically be the version tested on Travis CI since we are testing against HHVM latest.

avatar andrepereiradasilva
andrepereiradasilva - comment - 17 Oct 2016

so all we have to do is wait?

avatar photodude
photodude - comment - 17 Oct 2016

@andrepereiradasilva There is no harm in merging as it's a semantics change to our unit test, but yes it will also be fixed by just waiting.

I would suggest backing out the pg_set_error_verbosity() change if we do chose to merge, as that portion of the PR is no longer needed as of HHVM 3.15.2.

avatar mbabker
mbabker - comment - 17 Oct 2016

Depends what we consider a minimum supported HHVM version to be. If it's something older than 3.15.2 it'll be needed.

avatar photodude
photodude - comment - 17 Oct 2016

@mbabker Since hhvm 3.15.0 is the first version with pgsql.so in core I would suggest that would be the earlist LTS version to consider. 3.15.2 is also the first version were tests do not error out on memory.

As noted in #10220 (comment) we can pick up any recent LTS version of HHVM for testing on Travis CI or latest or nightly. we can also test with and without HHVM's php 7 mode (although the HHVM php 7 mode is currently broken due to facebook/hhvm#7198 )

We still need a fix for facebook/hhvm#2060 and to solve the JInstallerAdapterTest failures or determine if the JInstallerAdapterTest failures are an HHVM bug before we can determine a minimum HHVM version to support. I've been pushing HHVM to include the important bug fixes we need in the patches of 3.15 so that hopefully 3.15 can be a valid LTS release to target for a supported version.

avatar wilsonge
wilsonge - comment - 18 Oct 2016

Does the CMS actually work on HHVM practically (and by work I mean is the core extensions + functionality useable not just do the unit tests pass)

avatar mbabker
mbabker - comment - 18 Oct 2016

Who actually runs HHVM outside of Travis to test that? ?

avatar brianteeman
brianteeman - comment - 18 Oct 2016

It's a lot of work being done if no one uses hhvm

avatar photodude
photodude - comment - 18 Oct 2016

if anyone has a full siteground account, I believe that host does HHVM.

I've seen a few comments from people who tried running J3.4 on HHVM but ran into a lot of issues. Some of the past items have been fixed in the CMS or in HHVM.

At the moment the failures in the unit tests suggest to me that it's unlikely for the CMS to actually run on HHVM without issues. But I haven't tried running anything other than unit tests.

avatar brianteeman
brianteeman - comment - 18 Oct 2016

Only on their Cloud Hosting plans - not the regular ones

On 18 October 2016 at 15:12, Walt Sorensen notifications@github.com wrote:

if anyone has a full siteground account, I believe that host does HHVM.

I've seen a few comments from people who tried running J3.4 on HHVM but
ran into a lot of issues. Some of the past items have been fixed in the CMS
or in HHVM.

At the moment the failures in the unit tests suggest to me that it's
unlikely for the CMS to actually run on HHVM without issues. But I haven't
tried running anything other than unit tests.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#12336 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8T1pOOAU5aWOpl3IIw--VI8q6tVFks5q1NO3gaJpZM4KQx4U
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
https://brian.teeman.net/ http://brian.teeman.net/

avatar brianteeman
brianteeman - comment - 18 Oct 2016

and it is only an option not a default

On 18 October 2016 at 15:32, Brian Teeman brian@teeman.net wrote:

Only on their Cloud Hosting plans - not the regular ones

On 18 October 2016 at 15:12, Walt Sorensen notifications@github.com
wrote:

if anyone has a full siteground account, I believe that host does HHVM.

I've seen a few comments from people who tried running J3.4 on HHVM but
ran into a lot of issues. Some of the past items have been fixed in the CMS
or in HHVM.

At the moment the failures in the unit tests suggest to me that it's
unlikely for the CMS to actually run on HHVM without issues. But I haven't
tried running anything other than unit tests.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#12336 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8T1pOOAU5aWOpl3IIw--VI8q6tVFks5q1NO3gaJpZM4KQx4U
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
https://brian.teeman.net/ http://brian.teeman.net/

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
https://brian.teeman.net/ http://brian.teeman.net/

avatar photodude
photodude - comment - 18 Oct 2016

Best priced options for HHVM that I could find on a quick search was Google cloud at $5/mo or free trial and A2 Hosting at $5/mo

red hat openshift claims to have a free HHVM option in their developer preview, but I couldn't get it to launch into my account.

Heroku says it offers HHVM as a "highly experimental option", but I haven't had time to deal with a platform that relies heavily on command line for setup and activation. I was looking at heroku as an option since heroku has a free option that can be sandboxed for testing.

avatar brianteeman
brianteeman - comment - 18 Oct 2016

The question is "are there any users with hhvm"

On 18 October 2016 at 16:45, Walt Sorensen notifications@github.com wrote:

Best priced options for HHVM that I could find on a quick search was Google
cloud at $5/mo or free trial
https://console.cloud.google.com/launcher/details/bitnami-launchpad/hhvm
and A2 Hosting at $5/mo https://www.a2hosting.com/hhvm-hosting

red hat openshift claims to have a free HHVM option
https://hub.openshift.com/quickstarts/127-hhvm in their developer
preview, but I couldn't get it to launch into my account.

Heroku says it offers HHVM as a "highly experimental option"
https://devcenter.heroku.com/articles/php-support#selecting-a-runtime-hhvm,
but I haven't had time to deal with a platform that relies heavily on
command line for setup and activation. I was looking at heroku as an option
since heroku has a free option that can be sandboxed for testing
https://www.heroku.com/pricing.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#12336 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8XSbWbwK-xz0BnspQ4F-Tl_dMz_Qks5q1OmzgaJpZM4KQx4U
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
https://brian.teeman.net/ http://brian.teeman.net/

avatar photodude
photodude - comment - 18 Oct 2016

The question is "are there any users with hhvm"

As I said, I've seen a few comments from people who tried running J3.4.6 on HHVM but ran into a lot of issues. I haven't seen anything more recent.

avatar wilsonge wilsonge - change - 29 Oct 2016
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2016-10-29 20:48:51
Closed_By wilsonge
avatar wilsonge wilsonge - close - 29 Oct 2016
avatar wilsonge wilsonge - merge - 29 Oct 2016
avatar wilsonge wilsonge - reference | b3a112b - 29 Oct 16
avatar wilsonge wilsonge - merge - 29 Oct 2016
avatar wilsonge wilsonge - close - 29 Oct 2016
avatar photodude
photodude - comment - 30 Oct 2016

We still need the DSN separator portion of this backported to staging since #12341 was closed. (of course, it won't be needed whenever HHVM 3.16.0 gets released in the next few weeks or hopefully 3.15.3 if they merge the fix for the release in the next few weeks).

Add a Comment

Login with GitHub to post a comment