- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 334
 
swoole hooks can be compiled if pgsql/sqlite are not compiled in statically #864
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
Changes from 17 commits
e975e15
              779a724
              75f734d
              8104ff7
              7e0e909
              448941f
              b6d4af2
              93223a9
              1c7fa01
              324ba0d
              a46ad7b
              d13e369
              9fe3223
              99ccbf8
              2b57bca
              efdfbf4
              868f6d4
              65ee747
              f80aee5
              37e0f1d
              2694dd9
              b1da64d
              a1f2126
              ba32697
              9803bf6
              43352ab
              4eac953
              00f2625
              2d409db
              effefd4
              0d4d428
              d9c2247
              1243fb9
              c433aed
              00892c2
              ecdb94b
              08ab3c1
              3da58d5
              25401e5
              95f1b65
              5a4b920
              25fe794
              117a54d
              5323608
              fc7e8eb
              1a7bf2d
              20fbbb1
              f187250
              866ca26
              39a9840
              bf55db9
              0f00501
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -52,6 +52,15 @@ public function __construct(?BuilderBase $builder = null, array $options = []) | |
| */ | ||
| public function config(array $extensions = [], array $libraries = [], bool $include_suggest_ext = false, bool $include_suggest_lib = false): array | ||
| { | ||
| $extra_exts = []; | ||
| foreach ($extensions as $ext) { | ||
| $extra_exts = array_merge($extra_exts, Config::getExt($ext, 'ext-suggests', [])); | ||
| } | ||
| foreach ($extra_exts as $ext) { | ||
| if ($this->builder?->getExt($ext) && !in_array($ext, $extensions)) { | ||
| $extensions[] = $ext; | ||
| } | ||
| } | ||
| 
         
      Comment on lines
    
      +55
     to 
      +63
    
   
  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! Example: swoole suggests ext swoole-hook-pgsql. Swoole-hook-pgsql requires pdo and pgsql libs. If we get the config from swoole, it doesn't directly depend on pgsql, but when pgsql hook is enabled, it does. It's the same way where libraries become dependencies of libraries that suggest them, when they are enabled. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But  I also tested the difference with and without this code, and for  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
 Extension::buildShared calls SpcConfigUtil::config([$ext->name]). So it only passes  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean:  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. I made  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, that makes sense. My initial thought was to provide the full list of extensions in buildShared, but that seems more complicated than this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would not work because it would pull in definitions for symbols that should not be available (looking at you, postgresql, defining libc methods...)  | 
||
| [$extensions, $libraries] = DependencyUtil::getExtsAndLibs($extensions, $libraries, $include_suggest_ext, $include_suggest_lib); | ||
| 
     | 
||
| ob_start(); | ||
| 
          
            
          
           | 
    ||
Uh oh!
There was an error while loading. Please reload this page.