phar: problem with renaming implementation

From: Date: Fri, 21 Mar 2008 19:53:42 +0000
Subject: phar: problem with renaming implementation
Groups: php.pecl.dev 
Request: Send a blank email to pecl-dev+get-5230@lists.php.net to get a copy of this message
Hi folks, First, the bad news. The current implementation of renaming is fatally flawed. In searching for the source of the memleak, I found that what happens on renaming is this: 1) phar is added to manifest with the new name 2) phar is not removed from manifest with the old name on shutdown, we get PHP trying to free both the new and the old name, hence the double free and memleak. The only way around this is to completely copy the phar manifest, but then we have a problem with flushing and it gets really messy really fast. The good news is that I think there is a simpler way to do this. What we can do is the following: 1) flush any changes 2) use good old copy() to copy from the old to the new phar (i.e. from blah.phar to blah.phar.gz) 3) parse the new phar, creating a brand new unconnected manifest entry with the new name 4) set the compression for the archive (if any) 5) flush the new archive, replacing the copy()ed file (blah.phar.gz in my example). This way, the old archive is never touched, and the new one then exists. Here's some more bad news. Every method that renames will have to return a new phar object - there is no way to transparently change the path used by DirectoryIterator once it has been instantiated. As such, we need to reconsider the API. I'd like to suggest we drop most of the compression and conversion methods altogether from their current implementation. Instead, I'd like to create a factory method of sorts that accepts as arguments an existing Phar object, and some flags to tell it what to do (compress? convert to zip? etc.) sample code: <?php $phar = new Phar('blah.phar'); $newphar = $phar->convert(Phar::ZIP); $another = $phar->convert(Phar::TGZ); $third = $phar->convert(Phar::TAR); $fourth = $phar->convert(Phar::GZ); ?> The constants we would use would be Phar::ZIP, Phar::PHAR, Phar::TAR, Phar::BZ2, Phar::GZ, Phar::DATA and Phar::UNCOMPRESSED. We would allow Phar::TAR | PHAR::GZ or similar combos, but not Phar::ZIP | PHAR::GZ. Phar::DATA would be used to convert an active phar into a data-only phar (i.e. remove the alias and the stub from a tar or zip) and could be or'ed with Phar::TAR and its GZ/BZ2 constants, or with Phar::ZIP. compressAllFiles*() would go back to its original purpose of compressing individual files within the archive (and would throw an exception if called for tar-based phar archives). Finally, we need a method for grabbing the phar filename. Discussion should ensue, and I'm also curious whether you want to take this on Steph? It should result in MUCH simpler code and logic for our debugging purposes as well as in the end-user PHP level. Greg

« previous php.pecl.dev (#5230) next »