Skip to content

Commit e5660de

Browse files
authored
merge the local/remote source processing (#483)
- For both remote and local extensions, we use unified function to process the compilation of source code. - Now, when downloaded the remote extension, we will try the make the artifact manifest and the extension manifest be two different manifests. This is pre change to support general extension bundle. - existing `parent` is reused, all the full-compiled composer could be used as a general extension bundle. --------- Signed-off-by: wbpcode <wbphub@gmail.com>
1 parent 34a61da commit e5660de

16 files changed

Lines changed: 549 additions & 242 deletions

cli/cmd/genconfig.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func (g *GenConfig) Run(ctx context.Context, dirs *xdg.Directories, logger *slog
102102
}
103103

104104
// If we're only generating config to print to the stdout, we can skip building the extensions
105-
// but if we're exporting it, we need ot build to make sure the extension files exist.
105+
// but if we're exporting it, we need to build to make sure the extension files exist.
106106
downloaded, err := downloadExtensions(ctx, downloader, g.Extensions, true)
107107
if err != nil {
108108
return err

cli/cmd/genconfig_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ func TestGenConfigWriteConfig(t *testing.T) {
371371
logger = internaltesting.NewTLogger(t)
372372
dirs = &xdg.Directories{DataHome: t.TempDir()}
373373

374-
gpl = &extensions.Manifest{Name: extensions.GoPluginLoaderName, Type: extensions.TypeGo, CShared: true, Bundle: extensions.ComposerBundle, Version: "1.0.0"}
374+
gpl = &extensions.Manifest{Name: extensions.GoPluginLoaderName, Type: extensions.TypeGo, CShared: true, Parent: extensions.ComposerBundle, Version: "1.0.0"}
375375
gplLib = extensions.LocalCacheExtension(dirs, gpl) // resolves to dym/composer/1.0.0/libcomposer.so
376376
)
377377
require.NoError(t, os.MkdirAll(filepath.Dir(gplLib), 0o750))

cli/cmd/run.go

Lines changed: 116 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"maps"
1616
"net"
1717
"os"
18-
"path/filepath"
1918
"runtime"
2019
"slices"
2120
"strconv"
@@ -264,83 +263,27 @@ func downloadExtensions(ctx context.Context, downloader *extensions.Downloader,
264263

265264
switch artifact.ArtifactType {
266265
case extensions.ArtifactBinary:
267-
if artifact.Manifest.Type == extensions.TypeGo {
266+
if artifact.Manifest.Type == extensions.TypeGo && !artifact.Manifest.CShared {
268267
// Ensure the composer is downloaded before running any extensions that may depend on it.
269-
if err = extensions.CheckOrDownloadLibComposer(ctx, downloader, artifact.Manifest.ComposerVersion, extensions.ComposerArtifactLite); err != nil {
268+
if err = extensions.CheckOrDownloadLibComposer(ctx, downloader, artifact.Manifest.ComposerVersion,
269+
extensions.ComposerArtifactLite); err != nil {
270270
return nil, fmt.Errorf("failed to download libcomposer %s for extension %s: %w",
271271
artifact.Manifest.ComposerVersion, name, err)
272272
}
273+
composerManifest, _ := extensions.GetComposerManifest(downloader.Dirs, artifact.Manifest.ComposerVersion)
274+
if composerManifest != nil {
275+
extensions.ResolveVersionsWithParent(artifact.ExtensionManifest, composerManifest)
276+
}
273277
}
274-
downloaded = append(downloaded, artifact.Manifest)
275-
278+
artifact.ExtensionManifest.CShared = artifact.Manifest.CShared
279+
downloaded = append(downloaded, artifact.ExtensionManifest)
276280
case extensions.ArtifactSource:
277-
switch artifact.Manifest.Type {
278-
// If the downloaded artifact is the composer bundle, we need to find the path to the extension
279-
// inside the composer source tree.
280-
case extensions.TypeComposer:
281-
var manifest, composerManifest *extensions.Manifest
282-
extensionSrc := extensions.LocalCacheComposerExtensionSourceDir(downloader.Dirs, artifact.Manifest, name)
283-
if extensionSrc == "" {
284-
return nil, fmt.Errorf("invalid source artifact for Go extension %s: missing expected source directory: %s", name, artifact.Path)
285-
}
286-
downloader.Logger.Info("loading downloaded extension manifest", "path", extensionSrc)
287-
288-
manifest, err = extensions.LoadLocalManifest(filepath.Join(extensionSrc, "manifest.yaml"))
289-
if err != nil {
290-
return nil, fmt.Errorf("failed to load manifest for composer extension %s from source artifact %q: %w",
291-
name, extensionSrc, err)
292-
}
293-
composerManifestPath := filepath.Join(extensions.LocalCacheComposerSourceArtifactDir(downloader.Dirs, artifact.Manifest), "manifest.yaml")
294-
composerManifest, err = extensions.LoadLocalManifest(composerManifestPath)
295-
if err != nil {
296-
return nil, fmt.Errorf("failed to load composer parent manifest for composer extension %s from source artifact %q: %w",
297-
name, extensionSrc, err)
298-
}
299-
300-
extensions.ResolveVersionsWithParent(manifest, composerManifest)
301-
manifest.Remote = true // Mark the manifest as remote since it is from a downloaded artifact
302-
303-
if build {
304-
fmt.Printf("→ %sBuilding %s...%s\n", internal.ANSIBold, name, internal.ANSIReset)
305-
downloader.Logger.Info("building downloaded Go extension", "name", manifest.Name, "version", artifact.Manifest.Version)
306-
// Build libcomposer from the downloaded source if it does not exist in the local cache.
307-
if _, err = os.Stat(extensions.LocalCacheComposerLib(downloader.Dirs, artifact.Manifest.Version)); err != nil {
308-
if err = extensions.BuildLibComposer(downloader.Logger, downloader.Dirs, artifact.Path, artifact.Manifest.Version, false); err != nil {
309-
return nil, fmt.Errorf("failed to build libcomposer %s for extension %s: %w",
310-
artifact.Manifest.Version, name, err)
311-
}
312-
}
313-
if _, err = extensions.BuildExtensionFromPath(downloader.Logger, downloader.Dirs, manifest, extensionSrc); err != nil {
314-
return nil, fmt.Errorf("failed to build Go extension %s from source artifact: %w", name, err)
315-
}
316-
}
317-
318-
downloaded = append(downloaded, manifest)
319-
320-
case extensions.TypeRust:
321-
if build {
322-
fmt.Printf("→ %sBuilding %s...%s\n", internal.ANSIBold, name, internal.ANSIReset)
323-
downloader.Logger.Info("building downloaded Rust extension", "name", artifact.Manifest.Name, "version", artifact.Manifest.Version)
324-
325-
if err = extensions.CheckOrBuildDynamicModule(downloader.Logger, downloader.Dirs, artifact.Manifest, artifact.Path); err != nil {
326-
return nil, fmt.Errorf("failed to build Rust extension %s from source artifact: %w", name, err)
327-
}
328-
}
329-
downloaded = append(downloaded, artifact.Manifest)
330-
331-
case extensions.TypeExtProc:
332-
if build {
333-
fmt.Printf("→ %sBuilding %s...%s\n", internal.ANSIBold, name, internal.ANSIReset)
334-
downloader.Logger.Info("building downloaded ExtProc extension", "name", artifact.Manifest.Name, "version", artifact.Manifest.Version)
335-
if err = extensions.CheckOrBuildExtProcBinary(downloader.Logger, downloader.Dirs, artifact.Manifest, artifact.Path); err != nil {
336-
return nil, fmt.Errorf("failed to build ExtProc extension %s from source artifact: %w", name, err)
337-
}
338-
}
339-
downloaded = append(downloaded, artifact.Manifest)
340-
default:
341-
downloaded = append(downloaded, artifact.Manifest)
281+
handleSourceError := handleExtensionSource(ctx, downloader, artifact.Manifest, artifact.ExtensionManifest,
282+
artifact.Path, downloader.Logger, build)
283+
if handleSourceError != nil {
284+
return nil, handleSourceError
342285
}
343-
286+
downloaded = append(downloaded, artifact.ExtensionManifest)
344287
default:
345288
return nil, fmt.Errorf("unknown artifact type %q for extension %s", artifact.ArtifactType, name)
346289
}
@@ -373,7 +316,7 @@ func resolveGoPluginLoader(ctx context.Context, downloader *extensions.Downloade
373316
Name: extensions.GoPluginLoaderName,
374317
Type: extensions.TypeGo,
375318
CShared: true,
376-
Bundle: extensions.ComposerBundle,
319+
Parent: extensions.ComposerBundle,
377320
Version: version,
378321
ComposerVersion: version,
379322
// Marked remote so extensionPositions.sort can match it by reference, like
@@ -426,7 +369,9 @@ func parseLogLevels(logLevel string) (string, string, error) {
426369
var errFailedToLoadLocalManifest = errors.New("failed to load local manifest")
427370

428371
// loadLocalManifests loads extension manifests from the specified local paths.
429-
func loadLocalManifests(ctx context.Context, logger *slog.Logger, downloader *extensions.Downloader, paths []string, build bool) ([]*extensions.Manifest, error) {
372+
func loadLocalManifests(ctx context.Context, logger *slog.Logger, downloader *extensions.Downloader,
373+
paths []string, build bool,
374+
) ([]*extensions.Manifest, error) {
430375
manifests := make([]*extensions.Manifest, 0, len(paths))
431376

432377
for _, path := range paths {
@@ -437,55 +382,47 @@ func loadLocalManifests(ctx context.Context, logger *slog.Logger, downloader *ex
437382
return nil, fmt.Errorf("%w from %s: %w", errFailedToLoadLocalManifest, path, err)
438383
}
439384

385+
// This local extension may be a sub-extension of an extension bundle (e.g., a filter in the composer
386+
// source tree, a filter in Rust extension bundle and so on).
387+
// If so, we need to find the root manifest because we treat an extension bundle as a unit for version and
388+
// compilation management.
389+
var rootManifest *extensions.Manifest
390+
var rootPath string
440391
if manifest.Parent != "" {
441-
parent, err := resolveParent(ctx, downloader, manifest)
392+
rootManifest, rootPath, err = resolveParentNoFallback(manifest)
442393
if err != nil {
443394
return nil, fmt.Errorf("%w from %s: %w", errFailedToLoadLocalManifest, path, err)
444395
}
445-
extensions.ResolveVersionsWithParent(manifest, parent)
396+
extensions.ResolveVersionsWithParent(manifest, rootManifest)
446397
}
447-
448-
if build {
449-
switch manifest.Type {
450-
case extensions.TypeGo:
451-
fmt.Printf("→ %sBuilding %s...%s\n", internal.ANSIBold, manifest.Name, internal.ANSIReset)
452-
downloader.Logger.Info("building local Go extension", "name", manifest.Name, "version", manifest.Version)
453-
cshared, err := extensions.BuildExtensionFromPath(downloader.Logger, downloader.Dirs, manifest, path)
454-
if err != nil {
455-
return nil, err
456-
}
457-
if !cshared {
458-
// Old-style plugin needs libcomposer to load it.
459-
if err := extensions.DownloadLibComposerAndBuildIfNeeded(ctx, downloader, manifest.ComposerVersion, extensions.ComposerArtifactSource); err != nil {
460-
return nil, err
461-
}
462-
}
463-
manifest.CShared = cshared
464-
case extensions.TypeRust:
465-
fmt.Printf("→ %sBuilding %s...%s\n", internal.ANSIBold, manifest.Name, internal.ANSIReset)
466-
downloader.Logger.Info("building local Rust extension", "name", manifest.Name, "version", manifest.Version)
467-
// Build dynamic module (currently supports Rust)
468-
if err := extensions.BuildDynamicModule(downloader.Logger, downloader.Dirs, manifest, path); err != nil {
469-
return nil, err
470-
}
471-
case extensions.TypeExtProc:
472-
fmt.Printf("→ %sBuilding %s...%s\n", internal.ANSIBold, manifest.Name, internal.ANSIReset)
473-
downloader.Logger.Info("building local ext_proc extension", "name", manifest.Name, "version", manifest.Version)
474-
if err := extensions.BuildExtProcBinary(downloader.Logger, downloader.Dirs, manifest, path); err != nil {
475-
return nil, err
476-
}
477-
}
398+
if rootManifest == nil {
399+
rootManifest = manifest
400+
rootPath = path
401+
}
402+
err = handleExtensionSource(ctx, downloader, rootManifest, manifest, rootPath, logger, build)
403+
if err != nil {
404+
return nil, err
478405
}
479-
480406
manifests = append(manifests, manifest)
481407
}
482408

483409
return manifests, nil
484410
}
485411

412+
func resolveParentNoFallback(m *extensions.Manifest) (*extensions.Manifest, string, error) {
413+
parent, dir, err := extensions.FindLocalParentManifest(m)
414+
if err != nil {
415+
return nil, "", err
416+
}
417+
if parent != nil {
418+
return parent, dir, nil
419+
}
420+
return nil, "", fmt.Errorf("parent manifest %q not found locally for extension %s", m.Parent, m.Name)
421+
}
422+
486423
// resolveParent finds the parent manifest locally, falling back to the registry.
487424
func resolveParent(ctx context.Context, downloader *extensions.Downloader, m *extensions.Manifest) (*extensions.Manifest, error) {
488-
parent, err := extensions.FindLocalParentManifest(m)
425+
parent, _, err := extensions.FindLocalParentManifest(m)
489426
if err != nil {
490427
return nil, err
491428
}
@@ -543,11 +480,10 @@ func warnMultipleGoExtensions(manifests []*extensions.Manifest) {
543480
seen := make(map[string]bool)
544481
for _, ext := range manifests {
545482
if ext.Type == extensions.TypeGo && ext.CShared {
546-
name := ext.Name
547-
if ext.Bundle != "" {
548-
name = ext.Bundle
549-
}
550-
seen[name+":"+ext.Version] = true
483+
// ModuleName collapses bundle-hosted extensions (parent set, e.g.
484+
// goplugin-loader) onto the shared bundle library name, so several
485+
// extensions sharing one composer runtime count as a single library.
486+
seen[extensions.ModuleName(ext)+":"+ext.Version] = true
551487
}
552488
}
553489
if len(seen) < 2 {
@@ -587,3 +523,70 @@ func validateComposerCompat(manifests []*extensions.Manifest) error {
587523

588524
return nil
589525
}
526+
527+
// handleExtensionSource builds the bundle root from source (composer, Go, Rust or ext_proc) and
528+
// resolves the extension's runtime metadata (CShared, inherited versions) from the build result.
529+
// When build is false the build step is skipped entirely — used by config generation and tests
530+
// that only need manifest resolution, not compiled artifacts.
531+
func handleExtensionSource(ctx context.Context, downloader *extensions.Downloader, rootManifest *extensions.Manifest,
532+
extensionManifest *extensions.Manifest, rootPath string, logger *slog.Logger, build bool,
533+
) error {
534+
if !build {
535+
return nil
536+
}
537+
// The source artifact must be present on disk before we can build it. A missing directory
538+
// means the download produced no source tree, so fail fast with a clear message rather than
539+
// letting the underlying build tool fail obscurely (e.g. chdir into a non-existent path).
540+
if info, err := os.Stat(rootPath); err != nil || !info.IsDir() {
541+
return fmt.Errorf("source directory for extension %s does not exist: %s", rootManifest.Name, rootPath)
542+
}
543+
switch rootManifest.Type {
544+
case extensions.TypeComposer:
545+
fmt.Printf("→ %sBuilding composer for %s...%s\n", internal.ANSIBold, rootManifest.Name, internal.ANSIReset)
546+
logger.Info("building composer from local source", "name", rootManifest.Name, "version", rootManifest.Version)
547+
if err := extensions.BuildComposer(logger, downloader.Dirs, rootPath, rootManifest.Version); err != nil {
548+
return fmt.Errorf("failed to build libcomposer for local extension %s: %w", rootManifest.Name, err)
549+
}
550+
extensionManifest.CShared = true
551+
case extensions.TypeGo:
552+
fmt.Printf("→ %sBuilding %s...%s\n", internal.ANSIBold, rootManifest.Name, internal.ANSIReset)
553+
logger.Info("building local Go extension", "name", rootManifest.Name, "version", rootManifest.Version)
554+
cshared, err := extensions.BuildExtensionFromPath(logger, downloader.Dirs, rootManifest, rootPath)
555+
if err != nil {
556+
return fmt.Errorf("failed to build local Go extension %s: %w", rootManifest.Name, err)
557+
}
558+
if !cshared {
559+
// A locally-built old-style Go plugin is loaded via plugin.Open and must link against a
560+
// libcomposer built from the same source with the same toolchain/deps; a prebuilt lite
561+
// binary would be ABI-incompatible. Build from source unconditionally rather than via
562+
// CheckOrDownloadLibComposer: that cache is keyed only by version and shares the
563+
// dym/composer/<version>/libcomposer.so slot with the downloaded lite binary, so a cached
564+
// binary (from goplugin-loader or a downloaded plugin) would otherwise be reused here and
565+
// reintroduce the ABI mismatch.
566+
if err = extensions.DownloadLibComposerAndBuildIfNeeded(ctx, downloader, rootManifest.ComposerVersion,
567+
extensions.ComposerArtifactSource); err != nil {
568+
return fmt.Errorf("failed to build libcomposer %s for extension %s: %w",
569+
rootManifest.ComposerVersion, rootManifest.Name, err)
570+
}
571+
composerManifest, _ := extensions.GetComposerManifest(downloader.Dirs, rootManifest.ComposerVersion)
572+
if composerManifest != nil {
573+
extensions.ResolveVersionsWithParent(extensionManifest, composerManifest)
574+
}
575+
}
576+
extensionManifest.CShared = cshared
577+
case extensions.TypeRust:
578+
fmt.Printf("→ %sBuilding %s...%s\n", internal.ANSIBold, rootManifest.Name, internal.ANSIReset)
579+
downloader.Logger.Info("building local Rust extension", "name", rootManifest.Name, "version", rootManifest.Version)
580+
// Build dynamic module (currently supports Rust)
581+
if err := extensions.BuildDynamicModule(downloader.Logger, downloader.Dirs, rootManifest, rootPath); err != nil {
582+
return err
583+
}
584+
case extensions.TypeExtProc:
585+
fmt.Printf("→ %sBuilding %s...%s\n", internal.ANSIBold, rootManifest.Name, internal.ANSIReset)
586+
downloader.Logger.Info("building local ext_proc extension", "name", rootManifest.Name, "version", rootManifest.Version)
587+
if err := extensions.BuildExtProcBinary(downloader.Logger, downloader.Dirs, rootManifest, rootPath); err != nil {
588+
return err
589+
}
590+
}
591+
return nil
592+
}

0 commit comments

Comments
 (0)