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

Jump to bottom

archive.php compatibility with hhvm, next round #6224

Closed
voidswitch opened this issue Sep 16, 2014 · 4 comments
Closed

archive.php compatibility with hhvm, next round #6224

voidswitch opened this issue Sep 16, 2014 · 4 comments
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Milestone
2.8.0

Comments

@voidswitch
Copy link

After being fixed in #5263, the issue appears again in Piwik 2.6.1. findPhpBinary() incorrectly returns PHP_BINARY . ' -q' for hhvm because isValidPhpType() is true for /usr/bin/hhvm.

To workaround this issue I currently changed the order of checking isValidPhpType() and isHhvmBinary()

    public function findPhpBinary()
    {
        if (defined('PHP_BINARY')) {

            // check hhvm first
            if($this->isHhvmBinary(PHP_BINARY)) {
                return PHP_BINARY . ' --php';
            }

            // if not hhvm it shall not be fpm cgi or phpunit
            if($this->isValidPhpType(PHP_BINARY)) {
                return PHP_BINARY . ' -q';
            }
        }
# ...
@ThaDafinser
Copy link
Contributor

Maybe the hhvm test should be added to travis? See an example here (with allow failure)
https://github.com/ThaDafinser/ZfcDatagrid/blob/master/.travis.yml#L3-L15

@mattab
Copy link
Member

mattab commented Sep 17, 2014

thanks for the report!

Adding tests for HHVM would be really good! it is covered in this issue: Run Piwik with HipHop Virtual Machine - make all tests green on CI #4415

@mattab mattab added the Bug For errors / faults / flaws / inconsistencies etc. label Sep 17, 2014
@mattab mattab added this to the Piwik 2.8.0 milestone Sep 17, 2014
mattab added a commit that referenced this issue Oct 2, 2014
@mattab
Refs #6224 check for HHVM first to avoid it picked up by the isValidP…
6150aac 
…hpType call @voidswitch Does this fix the issue for you?
@mattab
Copy link
Member

mattab commented Oct 2, 2014

@voidswitch can you please confirm whether 6150aac fixes it for you?

@voidswitch
Copy link
Author

Yes, this fixes the problem. It's exactly what I did to workaround the problem. Thanks

@mattab mattab closed this as completed Oct 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Projects
None yet
Milestone
2.8.0
Development

No branches or pull requests
3 participants
@mattab @ThaDafinser @voidswitch