-
Notifications
You must be signed in to change notification settings - Fork 3k
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
HHVM doesn't support tar, tar.gz, tar.bz2 and zip versions of Phar archives #4263
Comments
One more detail - I've created those archives, renamed into file_get_contents('phar:///text.phar.php/text'); This phar:// wrapper works perfectly with PHP5. |
I found in commits that there should be support of compressed phar archives inside $phar = new PharData(__DIR__.'/text.tar.bz2', null, 'text.phar');
file_get_contents('phar://text.phar/text'); Last line causes:
While first line works fine. |
I'll take a look at this and see what I can do |
Phar considered corrupt, ref. scrutinizer-ci/ocular#10 and facebook/hhvm#4263
Do we see any chance that in near future phar archives will receive fully support and PHP compatibility? |
No immediate plans that I'm aware of. I might have time to take a look in the next couple days, but other stuff might come up, we'll see how it goes. |
[Above two comments deleted, they were on the wrong task, sorry.] |
Duplicate: #5139 "Phing phar outputs gibberish on hhvm" This is blocking use of compression by default* in Composer: composer/composer#3890 * typically wasting bandwidth, at least when using satis. However you might argue composer should not use phar for it's archives in the first place) |
Not sure that #1854 is really related, looks like totally different area. |
Yes, it was regarding missing compression support in general, but deleted that part now. |
Also, it does not seem that #5139 is related as the file in question seems not to be compressed. |
There is https://pear.php.net/package/PHP_Archive/ licensed under PHP License. Is it possible to just to include it into HHVM, register as stream wrapper for
It can be at least taken as foundation for further improvements since PHP License is fine for HHVM. I can try to bring it in, but I need some confirmation that this is actually good thing to do and it will be accepted in such form. |
No, we can't have that replace the existing Phar support, because that deliberately has a different API. It is reasonable to use that as a base to improve the existing Phar support though. |
Good, I'll try to do something about it. |
It is interesting that tar/zip have some initial support in Will PR when read support is ready, seems pretty doable as far as it is mostly PHP, not Hack/C 😄 |
Awesome! Ideally those docs changes should go in a separate PR, as they're far easier to review and are a lot of lines. (the format you switched them to is what the rest of the docs in the runtime use, so it's a good change) |
I've separated them into distinct commits intentionally. |
I'll decouple massive trivial PhpDoc changes separately and afterwards will rebase and submit another PR. More details will be in PRs. Feel free to test it in the mean time (and submit more tests) 💣 🚀 🌟 |
Summary: This is preparation for landing support for compressed Phar files. As discussed in #4263 changes that doesn't affect code logic separated for easier review. This PR primarily includes manual edits of PhpDoc sections to conform with arguments names from PHP documentation and to make them understandable to IDE. Also some Exceptions classes changed as it is defined in PHP docs. Closes #7019 Reviewed By: Orvid Differential Revision: D3221580 fb-gh-sync-id: f34bd8dabb7d507db862fe391e7213d90d487963 fbshipit-source-id: f34bd8dabb7d507db862fe391e7213d90d487963
#7028 it contains description and implementation of support for all kinds of compressed Phar and some additional stuff. |
Closing this as #7028 was merged |
tar.gz and tar.bz2 fails with:
tar fails with:
zip fails with:
Since all this variant supposed to be supported it is a huge incompatibility with PHP5 and should be fixed in next version.
The text was updated successfully, but these errors were encountered: