box-project / box

πŸ“¦πŸš€ Fast, zero config application bundler with PHARs.
https://box-project.github.io/box
MIT License
1.14k stars 100 forks source link

Suppress box req checker produces output in high verbosity even when successful #922

Open dkarlovi opened 1 year ago

dkarlovi commented 1 year ago

Bug report

Question Answer
Box version 4.2.0
PHP version 8.1.13
Platform with version Alpine
Github Repo https://github.com/sigwinhq/ariadne

When the PHAR is built, the req checker is built into it.

When I run my PHAR with (high) verbosity, I expect it to work like Box doesn't exist, if that's possible (reqs all pass).

Currently, the req checker will produce output when I pass either -vv or -vvv to my PHARed command. This should only be done if the req checker fails, but currently it happens either way.

box.json.dist ```json { "files": [ "config/autoload_runtime.php.template", "config/bundles.php", "config/services.yaml" ], "compression": "GZ" } ```
Output ```bash $ ariadne test goo -vvv Box Requirements Checker ======================== > Using PHP 8.2.3 > PHP is using the following php.ini file: /etc/php.ini > Checking Box requirements: βœ” The application requires the version "^8.1" or greater. βœ” The application requires the extension "zlib". βœ” The package "knplabs/github-api" requires the extension "json". βœ” The package "m4tthumphrey/php-gitlab-api" requires the extension "json". βœ” The package "m4tthumphrey/php-gitlab-api" requires the extension "xml". βœ” The package "symfony/framework-bundle" requires the extension "xml". [OK] Your system is ready to run the application. _ _ /\ (_) | | / \ _ __ _ __ _ __| | _ __ ___ / /\ \ | '__|| | / _` | / _` || '_ \ / _ \ / ____ \ | | | || (_| || (_| || | | || __/ /_/ \_\|_| |_| \__,_| \__,_||_| |_| \___| ! [NOTE] Using config: /home/dkarlovi/Development/Sigwin/Infra/ariadne.yaml ```
theofidry commented 1 year ago

I think it is still useful to display the logging for the requirement checker for debugging purposes though.

Would be putting via an environment variable acceptable?

dkarlovi commented 1 year ago

Would be putting via an environment variable acceptable?

Yeah, that would work. I don't think Box should react to the verbosity level passed to the app itself, that flags just happen to be the same. This should require a dedicated flag which can't be used on accident like this.

Thinking about it further, it still might make sense to be able to suppress it even with an env var. Say you in the future add the ability to make a self-executing PHAR (basically, Box creates exe files). I don't want my prod build to have Box debugging built in, the assumption is Box was used to compile the exe file, it's no longer used and shouldn't expose any "API".

But maybe in that case you can just disable it since you know the runtime? :thinking:

theofidry commented 1 year ago

Say you in the future add the ability to make a self-executing PHAR

In this case I think you make sure that the shipped PHP version contains the required versions upstream so you don't need the requirement checker at all.

Thinking about it further, it still might make sense to be able to suppress it even with an env var

Just to be clear: I meant that the env var dictates the verbosity instead, not suppress it. So if you don't pass it at all unless failing it would not show, but you keep the ability to display it in verbose mode (which I do need).

dkarlovi commented 1 year ago

Sounds good to me, it's off by default, but you can force it by using a Box specific env var. πŸ‘Œ