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

[PHP] HHVM xml parsing issues #2863

Closed
lsmith77 opened this issue Oct 14, 2014 · 28 comments · Fixed by travis-ci/travis-build#322
Closed

[PHP] HHVM xml parsing issues #2863

lsmith77 opened this issue Oct 14, 2014 · 28 comments · Fixed by travis-ci/travis-build#322

Comments

@lsmith77
Copy link

https://travis-ci.org/FriendsOfSymfony/FOSRestBundle/jobs/37943927
https://travis-ci.org/doctrine/DoctrinePHPCRBundle/jobs/37943272

not sure if this is just a bug in the version of HHVM or if there was an issue with how it was compiled.

@BanzaiMan
Copy link
Contributor

This looks like HHVM issue to me. Please report the issue to https://github.com/facebook/hhvm.

If the issue turns out to be on our end, we'll investigate.

@lsmith77
Copy link
Author

you may need to rollback to HHVM 3.2 then .. because this will break a lot of test suites

@lsmith77
Copy link
Author

opened facebook/hhvm#3974

@lsmith77
Copy link
Author

@dazz
Copy link

dazz commented Oct 15, 2014

Reproduced in the VM, also running with HHVM-3.3.0-0-g0a3cfb87b8a353fc7e1d15374f4adc413e37aba9

After adding the ini param from facebook/hhvm#3974 (comment) the ci runs localy without errors.

In /etc/hhvm/php.ini

hhvm.libxml.ext_entity_whitelist=file,http,https

@svenfuchs
Copy link
Contributor

This sounds like a compromise between rolling back and waiting for the next HHVM fix. I'll reopen this.

@BanzaiMan wdyt?

@svenfuchs svenfuchs reopened this Oct 15, 2014
@lsmith77
Copy link
Author

agreed

@meatballhat
Copy link

👍 to @dazz's patch to /etc/hhvm/php.ini iiuc

@BanzaiMan
Copy link
Contributor

Putting back /etc/hhvm/php.ini breaks hhvm-nightly. I guess I'll add a hack on travis-build, but, in all honesty, I feel that this is something HHVM team should fix.

@svenfuchs
Copy link
Contributor

Is there an alternative file location that hhvm would look at and that would take precedence over /etc/hhvm/php.ini? If so, users could just copy and modify the files in a before_script step.

@dazz
Copy link

dazz commented Oct 15, 2014

We added our own travis.hhvm.ini and call the ci commands for hhvm with the -c option to load our own config

hhvm -c travis.hhvm.ini ./vendor/bin/phpmd ...

@joshk
Copy link
Contributor

joshk commented Oct 16, 2014

@loicfrering hey buddy, do you think we could solve this with php-env? eg. when we install hhvm-nightly it would add an ini file as an after install step?

@BanzaiMan
Copy link
Contributor

I should mention that we tried tinkering with /etc/hhvm/php.ini before. See #2523.

@lsmith77
Copy link
Author

situation is a bit problematic atm as several projects now fail with HHVM. would you consider a rollback to HHVM 3.2? I really do not want to complicate our test setup with too much HHVM specific instructions. The other alternative is of course to just drop HHVM testing for now ..

/cc @sgolemon

@dbu
Copy link

dbu commented Oct 21, 2014

this hits about every symfony bundle doing tests, for example FriendsOfSymfony/FOSHttpCacheBundle#153

@stof
Copy link

stof commented Oct 21, 2014

@BanzaiMan isn't it possible to put this config back for HHVM stable at least ?

@BanzaiMan
Copy link
Contributor

I'll look into the hack today.

BanzaiMan added a commit to travis-ci/travis-build that referenced this issue Oct 21, 2014
BanzaiMan added a commit to travis-ci/travis-build that referenced this issue Oct 21, 2014
@BanzaiMan
Copy link
Contributor

I tested the suggested workaround, but it doesn't seem to solve a problem initially reported.

https://staging.travis-ci.org/BanzaiMan/FOSRestBundle/builds/392534#L160-L161 shows that this job has the desired entry in /etc/hhvm/php.ini, but subsequent tests still fail.

@stof
Copy link

stof commented Oct 21, 2014

there is a weird thing in the XSD path appearing in the error message. There is 4 slashes after file:, while there should be only 3 AFAICT. Maybe there is another bug as well

@BanzaiMan
Copy link
Contributor

The number of slashes after the protocol should not be a problem, as long as there are at least 3. In other words, file://// should collapse to file:///, since // is the same as / on the file system.

@BanzaiMan
Copy link
Contributor

If HHVM is not doing that, then, yes, that's a bug.

@lsmith77
Copy link
Author

lsmith77 commented Nov 6, 2014

this is a work around .. but its imho too messy to really be acceptable:
FriendsOfSymfony/FOSRestBundle#898

BanzaiMan added a commit to travis-ci/travis-build that referenced this issue Nov 6, 2014
We need to run this before paranoid_mode disables sudo

This fixes travis-ci/travis-ci#2863 and
travis-ci/travis-ci#2523.
@BanzaiMan
Copy link
Contributor

It's been shipped, so please test your HHVM jobs without the workaround. Thank you!

@lsmith77
Copy link
Author

lsmith77 commented Nov 7, 2014

looks good! https://travis-ci.org/FriendsOfSymfony/FOSRestBundle/jobs/40057391

thx for your work!

@Ocramius
Copy link

Ocramius commented Nov 7, 2014

Very helpful, thank you!

@stof
Copy link

stof commented Nov 7, 2014

hhm-nightly is still broken: https://travis-ci.org/phpspec/phpspec/jobs/40279263

@BanzaiMan
Copy link
Contributor

Sorry, the fix has been lost during a deploy. We'll fix this Monday.

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

Successfully merging a pull request may close this issue.

9 participants