-
-
Notifications
You must be signed in to change notification settings - Fork 331
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 19 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.