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

HHVM doesn't support tar, tar.gz, tar.bz2 and zip versions of Phar archives #4263

Closed
nazar-pc opened this issue Nov 18, 2014 · 19 comments
Closed

Comments

@nazar-pc
Copy link
Contributor

tar.gz and tar.bz2 fails with:

\nFatal error: Uncaught exception 'PharException' with message 'Corrupt phar, can't read 4 bytes starting at offset 1140' in ...

tar fails with:

\nFatal error: Uncaught exception 'PharException' with message 'Malformed manifest. Expected 0 bytes, got 40' in ...

zip fails with:

\nFatal error: Uncaught exception 'PharException' with message 'Not a phar: phar:///text.phar.php/text' in ...

Since all this variant supposed to be supported it is a huge incompatibility with PHP5 and should be fixed in next version.

@nazar-pc
Copy link
Contributor Author

One more detail - I've created those archives, renamed into text.phar.php and tried to get contents of them in such way:

file_get_contents('phar:///text.phar.php/text');

This phar:// wrapper works perfectly with PHP5.

@nazar-pc
Copy link
Contributor Author

I found in commits that there should be support of compressed phar archives inside PharData class, unfortunately I've faced error here as well, wrapper fails in such way (while remaining original file name):

$phar = new PharData(__DIR__.'/text.tar.bz2', null, 'text.phar');
file_get_contents('phar://text.phar/text');

Last line causes:

\nFatal error: Uncaught exception 'PharException' with message 'Not a phar: phar://text.phar/text' in ...

While first line works fine.

@mgottein
Copy link
Contributor

I'll take a look at this and see what I can do

@mwjames
Copy link

mwjames commented Jan 21, 2015

Do we see any chance that in near future phar archives will receive fully support and PHP compatibility?

@jwatzman
Copy link
Contributor

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.

@jwatzman
Copy link
Contributor

[Above two comments deleted, they were on the wrong task, sorry.]

@andrerom
Copy link
Contributor

Related: #1854 "ob_gzhandler not implemented"

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)

@nazar-pc
Copy link
Contributor Author

Not sure that #1854 is really related, looks like totally different area.

@andrerom
Copy link
Contributor

Yes, it was regarding missing compression support in general, but deleted that part now.

@bauerj
Copy link
Contributor

bauerj commented Apr 19, 2015

Also, it does not seem that #5139 is related as the file in question seems not to be compressed.

@nazar-pc
Copy link
Contributor Author

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 phar and forget about all issues with Phar?
There are some features marked as missing in docs:

The PHP_Archive class differs in approach from the Phar class in that it does not support the ArrayAccess interface for accessing internal files, and does not support iteration over contents using foreach(). However, full support for the phar:// stream wrapper, support of Phar-specific metadata setting and retrieval using PHP_Archive::getPharMetaData() and file-specific metadata using PHP_Archive::getFileMetaData(), zlib and bzip2 compression of individual files, and archive-wide SHA1/MD5 signature makes PHP_Archive just as powerful as the Phar extension for distributing phars.

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.

@Orvid
Copy link
Contributor

Orvid commented Apr 20, 2016

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.

@nazar-pc
Copy link
Contributor Author

Good, I'll try to do something about it.

@nazar-pc
Copy link
Contributor Author

nazar-pc commented Apr 22, 2016

It is interesting that tar/zip have some initial support in PharData already.
Working on support in Phar here: https://github.com/facebook/hhvm/compare/master...nazar-pc:phar-compression-support?diff=split&expand=1&name=phar-compression-support

Will PR when read support is ready, seems pretty doable as far as it is mostly PHP, not Hack/C 😄

@Orvid
Copy link
Contributor

Orvid commented Apr 22, 2016

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)

@nazar-pc
Copy link
Contributor Author

I've separated them into distinct commits intentionally.
There will be changes for PhpDoc sections for related classes, so I'll try to put all of that together afterwards.

@nazar-pc
Copy link
Contributor Author

.phar.gz, .phar.bz2, .phar.tar, .phar.tar.gz, .phar.tar.bz2, .phar.zip and all mentioned under generic .phar format are supported.
There are some basic tests to ensure things will work.

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) 💣 🚀 🌟

hhvm-bot pushed a commit that referenced this issue Apr 29, 2016
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
@nazar-pc
Copy link
Contributor Author

nazar-pc commented May 1, 2016

#7028 it contains description and implementation of support for all kinds of compressed Phar and some additional stuff.

@nazar-pc
Copy link
Contributor Author

Closing this as #7028 was merged

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.

8 participants