search

box-project / box

📦🚀 Fast, zero config application bundler with PHARs.
https://box-project.github.io/box
MIT License
1.13k stars 100 forks source link

Related to issue #1236 and RequirementChecker section #1237

Open llaville opened 11 months ago

llaville commented 11 months ago

Why don't we see all extensions declared on root composer.json file

See with GitHub repo :

Built online for the first time at https://github.com/overtrue/phplint/actions/runs/7056960071/job/19209802647

But when we check with BOX info command, we got : 


API Version: 1.1.0

Archive Compression: None
Files Compression: None

Signature: SHA-1
Signature Hash: C72ECCE5306E94D119732B10D34311C30F993AC4

Metadata: None

Timestamp: 1701414157 (2023-12-01T07:02:37+00:00)

RequirementChecker:
  Required:
  - PHP ^8.0 (root)
  - ext-json (root)
  Conflict:
  - ext-psr (symfony/service-contracts)

Contents: 459 files (2.36MB)

 // Use the --list|-l option to list the content of the PHAR.

Only json extension is displayed, but not mbstring. Is there a reason or is it just a bug ?

llaville commented 11 months ago

We can check it online with result https://github.com/overtrue/phplint/actions/runs/7057695166/job/19211827974

theofidry commented 11 months ago

Could there be that a mbstring poly fill is present? Thanks to the composer.lock file we remove all the covered extension requirements

llaville commented 11 months ago

Personnaly I'd like the Composer Unused solution that display it even if the symfony/polyfill-mbstring is present

devilbox@php-8.1.26 in /shared/backups/github/phplint $ /shared/backups/php/composer-unused.phar --version
composer-unused 0.8.11
devilbox@php-8.1.26 in /shared/backups/github/phplint $ /shared/backups/php/composer-unused.phar

Results
-------

Found 10 used, 0 unused, 0 ignored and 0 zombie packages

 Used packages
 ✓ php
 ✓ ext-json
 ✓ ext-mbstring
 ✓ symfony/cache (https://github.com/symfony/cache)
 ✓ symfony/console (https://github.com/symfony/console)
 ✓ symfony/event-dispatcher (https://github.com/symfony/event-dispatcher)
 ✓ symfony/finder (https://github.com/symfony/finder)
 ✓ symfony/options-resolver (https://github.com/symfony/options-resolver)
 ✓ symfony/process (https://github.com/symfony/process)
 ✓ symfony/yaml (https://github.com/symfony/yaml)

 Unused packages

 Ignored packages

 Zombies exclusions (did not match any package)
llaville commented 11 months ago

@theofidry I've another use case that I don't understand origin.

I've recently played with PHP Benchmarking framework v1.12.15

And I've noticed that php extensions declared does not match with the box info:general command

API Version: 1.1.0

Built with Box: 4.3.8@5534406

Archive Compression: None
Files Compression: GZ

Signature: SHA-1
Signature Hash: AC745626F21904EBC3B1FB30A428886EDF13F4B8

Metadata: None

Timestamp: 1701260649 (2023-11-29T12:24:09+00:00)

RequirementChecker:
  Required:
  - PHP ^8.1 (root)
  - ext-zlib (root)
  - ext-dom (root)
  - ext-json (root)
  - ext-pcre (root)
  - ext-reflection (root)
  - ext-spl (root)
  - ext-tokenizer (root)
  Conflict:
  - ext-psr (root)

Contents: 907 files (821.25KB)

 // Use the --list|-l option to list the content of the PHAR.

Extension zlib is shown as a root definition, while it's not the case in PHPBench composer.json

Confirmed by the composer why ext-zlib command that told us that 

There is no installed package depending on "ext-zlib"

Any idea of where it come from ?

theofidry commented 11 months ago

Extension zlib is shown as a root definition, while it's not the case in PHPBench composer.json

Probably from:

Files Compression: GZ

If it's GZ-compressed it requires ext-zlib.

Currently in .requirements.php the source key is filled by either the package name or null for the root. Maybe this could be changed in favour of 'compression algorithm' for this case for clarity

theofidry commented 10 months ago

@llaville do you think it would be good enough to show this as part of a command rather than the info? The difference would be that you compute the requirements before creating the PHAR, as opposed to the info which shows the requirements shipped. This would offer two benefits:

WDYT?

llaville commented 10 months ago

@theofidry On my project https://github.com/llaville/umlwriter/tree/3.4.0

By invoking composer why ext-zlib command, I get only

jawira/plantuml-encoding v1.1.0 requires ext-zlib (*)

But when I compile project with BOX 4.6.0@95b0d8e, and run box info command, I get : 

API Version: 1.1.0

Built with Box: 4.6.0@95b0d8e

Archive Compression: None
Files Compression: GZ

Signature: SHA-512
Signature Hash: DAC919F2C1208C8D9797B2AFB978C6BF3E4E843A976D359766AF9BA0C1D555C289A5A59AEFE921A6964ED00571876D112FB3B95BC534E5E0C86577FEB97E6AB2

Metadata: None

Timestamp: 1702541592 (2023-12-14T08:13:12+00:00)

RequirementChecker:
  Required:
  - PHP ^8.0 (root)
  - ext-zlib (root)
  - ext-zlib (jawira/plantuml-encoding)
  - ext-tokenizer (nikic/php-parser)
  - ext-json (roave/better-reflection)
  Conflict:
  - ext-psr (symfony/service-contracts)

Contents: 1548 files (12.27MB)

 // Use the --list|-l option to list the content of the PHAR.

As the zlib extension is not defined into the composer.json file of the project, the mention ext-zlib (root) lead to confusion for me !

I'd like to see rather mention like ext-zlib (suggested by humbug/box because files compression is enabled)

That will give something better understandable (but it's only my opinion)

RequirementChecker:
  Required:
  - PHP ^8.0 (root)
  - ext-zlib (suggested by humbug/box because files compression is enabled)
  - ext-zlib (jawira/plantuml-encoding)
  - ext-tokenizer (nikic/php-parser)
  - ext-json (roave/better-reflection)
  Conflict:
  - ext-psr (symfony/service-contracts)
llaville commented 10 months ago

Another use case that lead for me to a confusion, is when an extension is defined as root in the composer.json but did not shown on the RequirementChecker section of the box info, just because there is a polyfill installed.

RequirementChecker:
  Required:
  - PHP ^8.1 (root)
  - ext-dom (root)
  - ext-json (root)
  Conflict:
  - ext-psr (symfony/service-contracts)

On project https://github.com/overtrue/phplint/tree/9.1, the mbstring extension is declared, but does not appear on box info report. And polyfill is only installed by other dependencies, but not declared on root project.

With composer why symfony/polyfill-mbstring, we get on PHP 8.1 platform : 

symfony/console v6.4.1 requires symfony/polyfill-mbstring (~1.0)
symfony/string  v6.4.0 requires symfony/polyfill-mbstring (~1.0)

Compare with composer-unused report that did not list the polyfill (installed) but show the mbstring extension

Results
-------

Found 11 used, 0 unused, 0 ignored and 0 zombie packages

 Used packages
 ✓ php
 ✓ ext-dom
 ✓ ext-json
 ✓ ext-mbstring
 ✓ symfony/cache (https://github.com/symfony/cache)
 ✓ symfony/console (https://github.com/symfony/console)
 ✓ symfony/event-dispatcher (https://github.com/symfony/event-dispatcher)
 ✓ symfony/finder (https://github.com/symfony/finder)
 ✓ symfony/options-resolver (https://github.com/symfony/options-resolver)
 ✓ symfony/process (https://github.com/symfony/process)
 ✓ symfony/yaml (https://github.com/symfony/yaml)

 Unused packages

 Ignored packages

 Zombies exclusions (did not match any package)
theofidry commented 10 months ago

I'd like to see rather mention like ext-zlib (suggested by humbug/box because files compression is enabled)

Yes that can be added, not sure if with the exact workding. Note that this is explicitly said in the build output already.

Another use case that lead for me to a confusion, is when an extension is defined as root in the composer.json but did not shown on the RequirementChecker section of the box info, just because there is a polyfill installed.

I totally understand the point, but in https://github.com/box-project/box/issues/1237#issuecomment-1855345713, my question is if having it as part of another command would be good enough.

The reason why I suggest another command is that it's a lot easier and cheaper to add information as part of a Box command. The produced requirement checker is shipped as part of the final PHAR so I prefer to keep the shipped code as stable and light as possible. Which is why listing all the intermediate steps of finding the requirements makes me a bit uneasy.