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

fix: Add back the old PharInfo #1067

Merged
merged 1 commit into from
Oct 14, 2023
Merged

Conversation

theofidry
Copy link
Member

Closes #1065.

@theofidry
Copy link
Member Author

/cc @llaville

@theofidry theofidry merged commit 08ca5f4 into box-project:main Oct 14, 2023
182 of 191 checks passed
@theofidry theofidry deleted the fix/bc-break branch October 14, 2023 22:18
@llaville
Copy link
Contributor

@theofidry Sorry to said that, but the BC break is not fully fixed. Let me try to explain.

With v4.3, the PharInfo class constructor (https://github.com/box-project/box/blob/4.3.8/src/PharInfo/PharInfo.php#L40-L55) was just a wrapped around native PHP Phar or PharData classes

With v4.5, the old class constructor (https://github.com/box-project/box/blob/4.5.0/src/PharInfo/PharInfo.php#L34-L37) is a wrapper to your new class that call an extract additionally command.

@theofidry
Copy link
Member Author

@llaville could you explain what is the BC break though? Because what is inside __construct() in itself is internal, in itself changing anything there does not constitute a BC break (as long as the behaviour remains the same – that's not the case since you report an issue, but I can't help if you tell me what the problem is)

@llaville
Copy link
Contributor

@theofidry I think we are agree on Backward Compatibility break definition in theory !

So If you compare both implementations :

That is if you compare these both implementations

In practical, both scripts with same call should gave same results whatever implementation have changed or not.

Look at these two scripts with output results inside (run on PHP 8.1.24)

<?php
// for BOX v4.3

require_once 'vendor/autoload.php';

use KevinGH\Box\PharInfo\PharInfo;

$pharFile = $argv[1];

$phar = (new PharInfo($pharFile))->getPhar();

\var_dump($phar);

/*
object(Phar)#7 (4) {
  ["pathName":"SplFileInfo":private]=>
  string(44) "phar:///shared/backups/php/box-4.3.phar/.box"
  ["fileName":"SplFileInfo":private]=>
  string(4) ".box"
  ["glob":"DirectoryIterator":private]=>
  bool(false)
  ["subPathName":"RecursiveDirectoryIterator":private]=>
  string(0) ""
}
 */
<?php
// for BOX v4.5
require_once 'vendor/autoload.php';

use KevinGH\Box\PharInfo\PharInfo;

$pharFile = $argv[1];

$phar = (new PharInfo($pharFile))->getPhar();

\var_dump($phar);

/*

Fatal error: Uncaught Symfony\Component\Process\Exception\ProcessFailedException: The command "'/usr/local/bin/php' 'pharInfo-45.php' 'extract' '/shared/backups/php/box-4.3.phar' '/tmp/HumbugBox/Pharaoh77091' '--no-interaction' '--internal'" failed.

Exit Code: 255(Unknown error)

Working directory: /shared/backups/bartlett/box-manifest

Output:
================

Fatal error: Uncaught KevinGH\Box\Phar\InvalidPhar: Could not find the file "extract". in /shared/backups/bartlett/box-manifest/vendor/humbug/box/src/Phar/InvalidPhar.php:52
Stack trace:
#0 /shared/backups/bartlett/box-manifest/vendor/humbug/box/src/Phar/PharInfo.php(99): KevinGH\Box\Phar\InvalidPhar::fileNotFound('extract')
#1 /shared/backups/bartlett/box-manifest/vendor/humbug/box/src/PharInfo/PharInfo.php(36): KevinGH\Box\Phar\PharInfo->__construct('extract')
#2 /shared/backups/bartlett/box-manifest/pharInfo-45.php(9): KevinGH\Box\PharInfo\PharInfo->__construct('extract')
#3 {main}
  thrown in /shared/backups/bartlett/box-manifest/vendor/humbug/box/src/Phar/InvalidPhar.php on line 52


Error Output:
================
 in /shared/backups/bartlett/box-manifest/vendor/humbug/box/src/Phar/PharInfo.php:304
Stack trace:
#0 /shared/backups/bartlett/box-manifest/vendor/humbug/box/src/Phar/PharInfo.php(114): KevinGH\Box\Phar\PharInfo::dumpPhar('/shared/backups...', '/tmp/HumbugBox/...')
#1 /shared/backups/bartlett/box-manifest/vendor/humbug/box/src/PharInfo/PharInfo.php(36): KevinGH\Box\Phar\PharInfo->__construct('/shared/backups...')
#2 /shared/backups/bartlett/box-manifest/pharInfo-45.php(9): KevinGH\Box\PharInfo\PharInfo->__construct('/shared/backups...')
#3 {main}

Next KevinGH\Box\Phar\InvalidPhar in /shared/backups/bartlett/box-manifest/vendor/humbug/box/src/Phar/PharInfo.php:301
Stack trace:
#0 /shared/backups/bartlett/box-manifest/vendor/humbug/box/src/Phar/PharInfo.php(114): KevinGH\Box\Phar\PharInfo::dumpPhar('/shared/backups...', '/tmp/HumbugBox/...')
#1 /shared/backups/bartlett/box-manifest/vendor/humbug/box/src/PharInfo/PharInfo.php(36): KevinGH\Box\Phar\PharInfo->__construct('/shared/backups...')
#2 /shared/backups/bartlett/box-manifest/pharInfo-45.php(9): KevinGH\Box\PharInfo\PharInfo->__construct('/shared/backups...')
#3 {main}
  thrown in /shared/backups/bartlett/box-manifest/vendor/humbug/box/src/Phar/PharInfo.php on line 301

 */

@llaville
Copy link
Contributor

But don't worry about this, as I've already said BOX Manifest have stopped to support BOX new versions after v4.3.8
I've just give a try to explore BOX v4.5

@theofidry
Copy link
Member Author

Ha because you're invoking that outside of the Box context I see.

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

Successfully merging this pull request may close these issues.

Why don't you choose to introduce BC breaks with BOX 4.4.0
2 participants