From 02a95c4783f3303f2bc9bf8ea2c647e0350f1fa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Duchesneau?= Date: Fri, 13 Dec 2024 11:36:49 -0500 Subject: [PATCH] fix protobuf descriptors topological ordering when writing spkg to prevent breaking the js client, prep release --- cmd/substreams/pack.go | 15 ++++++++----- docs/release-notes/change-log.md | 11 ++++++++++ manifest/reader.go | 37 ++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 5 deletions(-) diff --git a/cmd/substreams/pack.go b/cmd/substreams/pack.go index 997e8a01..7527427f 100644 --- a/cmd/substreams/pack.go +++ b/cmd/substreams/pack.go @@ -51,10 +51,6 @@ func runPack(cmd *cobra.Command, args []string) error { return fmt.Errorf("manifest reader: %w", err) } - if !manifestReader.IsLocalManifest() { - return fmt.Errorf(`"pack" can only be used to pack local manifest file`) - } - pkgBundle, err := manifestReader.Read() if err != nil { return fmt.Errorf("reading manifest %q: %w", manifestPath, err) @@ -69,9 +65,18 @@ func runPack(cmd *cobra.Command, args []string) error { originalOutputFile, _ := sflags.GetString(cmd, "output-file") + manifestDir := filepath.Dir(manifestPath) + if manifestReader.IsRemotePackage(manifestPath) { + manifestDir = "." + + if !manifestReader.IsLocalManifest() { + fmt.Printf("Re-packaging existing .spkg file...") + } + } + packageMetadata := pkgBundle.Package.PackageMeta[0] resolvedOutputFile := resolveOutputFile(originalOutputFile, map[string]string{ - "manifestDir": filepath.Dir(manifestPath), + "manifestDir": manifestDir, "spkgDefaultName": fmt.Sprintf("%s-%s.spkg", strings.Replace(packageMetadata.Name, "_", "-", -1), packageMetadata.Version), "version": packageMetadata.Version, }) diff --git a/docs/release-notes/change-log.md b/docs/release-notes/change-log.md index 27c616a4..d3a485d6 100644 --- a/docs/release-notes/change-log.md +++ b/docs/release-notes/change-log.md @@ -11,6 +11,17 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## v1.11.2 +### Server-side + +* Fix too many memory allocations impacting performance when stores are used + +### CLI + +* Force topological ordering of protobuf descriptors when 'packing' an spkg (affecting current substreams-js clients) +* Allow `substreams pack` to be able to do a "re-packing" of an existing spkg file. Useful to apply the protobuf descriptor ordering fix. + +### Docker image + * Rebuilt of v1.11.1 to generate Docker `latest` tag with revamp Docker image building. * Substreams CLI is now built with using Ubuntu 22, previous releases were built using Ubuntu 20. * Substreams Docker image is now using `ubuntu:22` as its base, previous releases were built using `ubuntu:20.04`. diff --git a/manifest/reader.go b/manifest/reader.go index 4af64f71..39229b11 100644 --- a/manifest/reader.go +++ b/manifest/reader.go @@ -19,6 +19,7 @@ import ( "github.com/streamingfast/dstore" "golang.org/x/mod/semver" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/descriptorpb" "github.com/jhump/protoreflect/desc" "github.com/streamingfast/cli" @@ -591,10 +592,46 @@ func (r *Reader) resolvePkg() (*pbsubstreams.Package, *Manifest, error) { return nil, nil, fmt.Errorf("unable to get package: %w", err) } + pkg.ProtoFiles, err = orderProtobufFilesByDependencies(pkg.ProtoFiles) + if err != nil { + return nil, nil, fmt.Errorf("unable to order protobuf files: %w", err) + } + r.pkg = pkg return pkg, manif, nil } +func orderProtobufFilesByDependencies(in []*descriptorpb.FileDescriptorProto) ([]*descriptorpb.FileDescriptorProto, error) { + var out []*descriptorpb.FileDescriptorProto + seen := make(map[string]bool) + for _, fd := range in { + if err := orderProtobufFilesByDependenciesRec(fd, in, seen, &out); err != nil { + return nil, err + } + } + return out, nil +} + +func orderProtobufFilesByDependenciesRec(fd *descriptorpb.FileDescriptorProto, in []*descriptorpb.FileDescriptorProto, seen map[string]bool, out *[]*descriptorpb.FileDescriptorProto) error { + if seen[fd.GetName()] { + return nil + } + seen[fd.GetName()] = true + + for _, dep := range fd.GetDependency() { + for _, d := range in { + if d.GetName() == dep { + if err := orderProtobufFilesByDependenciesRec(d, in, seen, out); err != nil { + return err + } + } + } + } + + *out = append(*out, fd) + return nil +} + // IsRemotePackage determines if reader's input to read the manifest is a remote file accessible over // HTTP/HTTPS, Google Cloud Storage, S3 or Azure Storage. func (r *Reader) IsRemotePackage(input string) bool {