Skip to content

Commit d49d545

Browse files
fix(plugins): address review feedback on unified loader
- Use copy_plugin helper for gain-native instead of raw cp + glob, ensuring consistent handling of versioned .so.* and plugin.yml. - Log a warning when the native plugins directory cannot be read, matching the behavior of load_active_plugin_records. - Distinguish 'already loaded' skips (debug level) from genuine load failures (warn level) to avoid misleading log output. - Sort discovered library paths for deterministic load order. - Add comment explaining the source-label heuristic. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
1 parent 3b396a5 commit d49d545

File tree

2 files changed

+41
-19
lines changed

2 files changed

+41
-19
lines changed

apps/skit/src/plugins.rs

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -273,36 +273,54 @@ impl UnifiedPluginManager {
273273
let mut dir_bundle_libs: Vec<std::path::PathBuf> = Vec::new();
274274
let mut bare_libs: Vec<std::path::PathBuf> = Vec::new();
275275

276-
if let Ok(entries) = std::fs::read_dir(&self.native_directory) {
277-
for entry in entries.flatten() {
278-
let path = entry.path();
279-
280-
if path.is_dir() {
281-
// Scan one level of subdirectories for plugin libraries.
282-
if let Ok(sub_entries) = std::fs::read_dir(&path) {
283-
for sub_entry in sub_entries.flatten() {
284-
let sub_path = sub_entry.path();
285-
if Self::is_native_lib(&sub_path) {
286-
dir_bundle_libs.push(sub_path);
276+
match std::fs::read_dir(&self.native_directory) {
277+
Ok(entries) => {
278+
for entry in entries.flatten() {
279+
let path = entry.path();
280+
281+
if path.is_dir() {
282+
// Scan one level of subdirectories for plugin libraries.
283+
if let Ok(sub_entries) = std::fs::read_dir(&path) {
284+
for sub_entry in sub_entries.flatten() {
285+
let sub_path = sub_entry.path();
286+
if Self::is_native_lib(&sub_path) {
287+
dir_bundle_libs.push(sub_path);
288+
}
287289
}
288290
}
291+
continue;
289292
}
290-
continue;
291-
}
292293

293-
if Self::is_native_lib(&path) {
294-
bare_libs.push(path);
294+
if Self::is_native_lib(&path) {
295+
bare_libs.push(path);
296+
}
295297
}
296-
}
298+
},
299+
Err(err) => {
300+
warn!(
301+
error = %err,
302+
dir = %self.native_directory.display(),
303+
"Failed to read native plugins directory"
304+
);
305+
},
297306
}
298307

308+
// Sort for deterministic load order when multiple bundles or bare
309+
// files provide the same plugin kind.
310+
dir_bundle_libs.sort();
311+
bare_libs.sort();
312+
299313
// Load directory-bundle plugins first, then bare files. Skip any
300314
// plugin whose kind is already loaded (from active records or an
301315
// earlier library in this phase).
302316
let mut summaries = Vec::new();
303317
for path in dir_bundle_libs.into_iter().chain(bare_libs) {
304318
match self.load_native_plugin(&path) {
305319
Ok(summary) => {
320+
// Classify the source based on the library's parent
321+
// directory: if it is directly inside the native dir it
322+
// came from a bare flat file; otherwise it is nested
323+
// inside a subdirectory (directory-bundle layout).
306324
let source = if path.parent() == Some(self.native_directory.as_path()) {
307325
"bare-file"
308326
} else {
@@ -317,7 +335,12 @@ impl UnifiedPluginManager {
317335
summaries.push(summary);
318336
},
319337
Err(err) => {
320-
warn!(error = %err, file = ?path, "Failed to load native plugin from disk");
338+
let msg = err.to_string();
339+
if msg.contains("already loaded") || msg.contains("already registered") {
340+
debug!(file = ?path, "Skipping plugin (already loaded by higher-priority source)");
341+
} else {
342+
warn!(error = %err, file = ?path, "Failed to load native plugin from disk");
343+
}
321344
},
322345
}
323346
}

justfile

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,8 +1038,7 @@ copy-plugins-native:
10381038
}
10391039

10401040
# Examples (gain-native has its own target dir, not shared)
1041-
mkdir -p .plugins/native/gain-native
1042-
cp examples/plugins/gain-native/target/release/libgain_plugin_native.* .plugins/native/gain-native/ 2>/dev/null || true
1041+
copy_plugin "gain-native" "gain_plugin_native" "examples/plugins/gain-native/target"
10431042

10441043
# Official native plugins (shared target dir).
10451044
# For most plugins the lib stem matches the plugin id.

0 commit comments

Comments
 (0)