Skip to content

Enhance exception handling by binding builder and extra info to exception handler #853

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

Merged
merged 14 commits into from
Aug 16, 2025

Conversation

crazywhalecc
Copy link
Owner

What does this PR do?

Fix #852

Checklist before merging

If your PR involves the changes mentioned below and completed the action, please tick the corresponding option.
If a modification is not involved, please skip it directly.

  • If you modified *.php or *.json, run them locally to ensure your changes are valid:
    • composer cs-fix
    • composer analyse
    • composer test
    • bin/spc dev:sort-config
  • If it's an extension or dependency update, please ensure the following:
    • Add your test combination to src/globals/test-extensions.php.
    • If adding new or fixing bugs, add commit message containing extension test or test extensions to trigger full test suite.

@crazywhalecc crazywhalecc requested a review from henderkes August 11, 2025 02:49
Copy link
Collaborator

@henderkes henderkes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bind is a verb like set is.

@crazywhalecc crazywhalecc requested a review from henderkes August 12, 2025 01:39
@henderkes
Copy link
Collaborator

henderkes commented Aug 12, 2025

Could you trigger a ./configure command failure instead please?

I also think that a lot of the currently shown options are useless.

Builder options:
    with-libs:                 ""
    build-shared:              ""
 --- useless unless crashing during specific sapi build
    build-micro:               true
    build-cli:                 true
    build-fpm:                 true
    build-embed:               false
    build-frankenphp:          false
    build-all:                 false
--- end useless
    no-strip:                  false # useless because stripping is safe
    disable-opcache-jit:       false
    with-config-file-path:     /usr/local/etc/php # useless
    with-config-file-scan-dir: /usr/local/etc/php/conf.d # useless
    with-hardcoded-ini:        [] # useless
    with-micro-fake-cli:       false
    with-suggested-libs:       false
    with-suggested-exts:       false
    with-added-patch:          []
    without-micro-ext-test:    false
    with-upx-pack:             false
    with-micro-logo:           null
    enable-micro-win32:        false
    debug:                     true # useless
    no-motd:                   false # useless
    preserve-log:              false
    with-clean:                false # difference to rebuild?
    rebuild:                   false
    enable-zts:                true
    help:                      false # useless
    silent:                    false # useless
    quiet:                     false # useless
    verbose:                   false # useless
    version:                   false # we already see the php version earlier in the log, also, why is this a thing in the builder? it only concerns the download, no?
    ansi:                      null # what is this?
    no-interaction:            false # useless

@henderkes
Copy link
Collaborator

The options shown right above contain everything we need I believe:

Build PHP extra info:
    Build OS:               Linux (x86_64)
    Build Target:           false
    Build Toolchain:        SPC\toolchain\MuslToolchain
    Build SAPI:             cli, micro, fpm
    Static Extensions (30): bcmath,bz2,calendar,ctype,curl,dom,exif,fileinfo,filter,ftp,iconv,xml,mbstring,mbregex,mysqlnd,zlib,openssl,pdo,pdo_mysql,sqlite3,pdo_sqlite,phar,session,simplexml,soap,sockets,tokenizer,xmlwriter,xmlreader,zip
    Shared Extensions (0):  ""
    Libraries (13):         pkg-config,bzip2,zlib,openssl,libiconv,libxml2,curl,onig,sqlite,libzip
    Strip Binaries:         yes
    Enable ZTS:             yes
    Config File Path:       /usr/local/etc/php
    PHP Version:            unknown

Builder function: proveExts

Just need to add if opcache-jit was disabled or not.

@crazywhalecc
Copy link
Owner Author

crazywhalecc commented Aug 12, 2025

I also think that a lot of the currently shown options are useless.

This just prints all symfony command arguments directly. I don't want to show all of them either but it's hard to pick and remove the default values only. Currently it will output to console only if we are using --debug option, and without it it will not be shown and only be logged in file. Honestly I almost never use --debug after updating the exception enhancement, because it is more convenient to view the log directly and console is clean with key error msg only.

Alternatively we could replace them to implode(' ', $_SERVER['argv']).

@crazywhalecc
Copy link
Owner Author

I noticed the spc.output.log contains argv debug log at the beginning. So removing it.

@henderkes
Copy link
Collaborator

Looks like the git master doesn't contain some of the header files. They are present in the php 8.X branches.

@henderkes
Copy link
Collaborator

https://downloads.php.net/~edorian/ beta 1 was released there. It also doesn't contain the header files.

@henderkes
Copy link
Collaborator

php/php-src#19483

@crazywhalecc
Copy link
Owner Author

php/php-src#19483

This is not related to this PR. I'll track them later in #842 . This could be merge now if nothing wrong.

@crazywhalecc crazywhalecc merged commit 5f62925 into main Aug 16, 2025
126 of 148 checks passed
@crazywhalecc crazywhalecc deleted the fix/exception-reflection branch August 16, 2025 11:33
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.

TypeError in SPCException
2 participants