From e554374f899512745db52aae4ab58778e806df6c Mon Sep 17 00:00:00 2001
From: jspc <james+git@zero-internet.org.uk>
Date: Fri, 11 Mar 2022 14:38:28 +0000
Subject: [PATCH 1/2] Install multiple packages together as a meta package

Because we now support triggers, and because triggers need to run at the absolute end, and because the potential install loop is a pain, we can install build a meta package when many packages are set. So long as that package stays out of the state db, so much the better.

See: https://github.com/vinyl-linux/vin/issues/44
---
 client/cmd/install.go  | 13 +------
 client/cmd/vin.go      |  4 +--
 client/cmd/vin_test.go | 14 ++++----
 manifest.go            |  4 +++
 manifest_db.go         | 18 +++++++++-
 proto/install.proto    |  2 +-
 server.go              | 80 +++++++++++++++++++++++++++++++++++-------
 server/install.pb.go   | 12 +++----
 server_test.go         | 29 ++++++++-------
 9 files changed, 121 insertions(+), 55 deletions(-)

diff --git a/client/cmd/install.go b/client/cmd/install.go
index efa21a4..5446b89 100644
--- a/client/cmd/install.go
+++ b/client/cmd/install.go
@@ -56,12 +56,6 @@ var installCmd = &cobra.Command{
 			return fmt.Errorf("missing package(s)")
 		}
 
-		if argCount > 1 && (version != "" && version != "latest") {
-			cmd.Usage()
-
-			return fmt.Errorf("setting version with multiple packages makes no sense")
-		}
-
 		return nil
 	},
 	RunE: func(cmd *cobra.Command, args []string) (err error) {
@@ -72,12 +66,7 @@ var installCmd = &cobra.Command{
 			return err
 		}
 
-		for _, pkg := range args {
-			err = client.install(pkg, version, force)
-			if err != nil {
-				break
-			}
-		}
+		err = client.install(args, version, force)
 
 		return errWrap("installation", err)
 	},
diff --git a/client/cmd/vin.go b/client/cmd/vin.go
index 6f1c44b..bce2ed0 100644
--- a/client/cmd/vin.go
+++ b/client/cmd/vin.go
@@ -63,9 +63,9 @@ func parseAddr(addr string) (s string, err error) {
 	return u.String(), nil
 }
 
-func (c client) install(pkg, version string, force bool) (err error) {
+func (c client) install(pkgs []string, version string, force bool) (err error) {
 	is := &vin.InstallSpec{
-		Pkg:   pkg,
+		Pkg:   pkgs,
 		Force: force,
 	}
 
diff --git a/client/cmd/vin_test.go b/client/cmd/vin_test.go
index 2ea0f5d..fa1d559 100644
--- a/client/cmd/vin_test.go
+++ b/client/cmd/vin_test.go
@@ -115,20 +115,20 @@ func (c *dummyInstallClient) getOutput() []string           { return c.output }
 func TestClient_Install(t *testing.T) {
 	for _, test := range []struct {
 		name         string
-		pkg          string
+		pkg          []string
 		ver          string
 		client       DummyVinClient
 		expectSpec   *vin.InstallSpec
 		expectOutput []string
 		expectError  bool
 	}{
-		{"valid package, 'latest' version", "foo", "latest", &dummyInstallClient{}, &vin.InstallSpec{Pkg: "foo"}, []string{}, false},
-		{"valid package, empty version", "foo", "", &dummyInstallClient{}, &vin.InstallSpec{Pkg: "foo"}, []string{}, false},
-		{"valid package, set version", "foo", "1.0.0", &dummyInstallClient{}, &vin.InstallSpec{Pkg: "foo", Version: "1.0.0"}, []string{}, false},
-		{"valid package, 'latest' version, output", "foo", "latest", &dummyInstallClient{output: []string{"line-1", "line-2"}}, &vin.InstallSpec{Pkg: "foo"}, []string{"line-1", "line-2"}, false},
+		{"valid package, 'latest' version", []string{"foo"}, "latest", &dummyInstallClient{}, &vin.InstallSpec{Pkg: []string{"foo"}}, []string{}, false},
+		{"valid package, empty version", []string{"foo"}, "", &dummyInstallClient{}, &vin.InstallSpec{Pkg: []string{"foo"}}, []string{}, false},
+		{"valid package, set version", []string{"foo"}, "1.0.0", &dummyInstallClient{}, &vin.InstallSpec{Pkg: []string{"foo"}, Version: "1.0.0"}, []string{}, false},
+		{"valid package, 'latest' version, output", []string{"foo"}, "latest", &dummyInstallClient{output: []string{"line-1", "line-2"}}, &vin.InstallSpec{Pkg: []string{"foo"}}, []string{"line-1", "line-2"}, false},
 
-		{"vind throws error", "foo", "1.0.0", &dummyInstallClient{err: true}, &vin.InstallSpec{Pkg: "foo", Version: "1.0.0"}, []string{}, true},
-		{"vind stream error", "foo", "1.0.0", &dummyInstallClient{recvErr: true}, &vin.InstallSpec{Pkg: "foo", Version: "1.0.0"}, []string{}, true},
+		{"vind throws error", []string{"foo"}, "1.0.0", &dummyInstallClient{err: true}, &vin.InstallSpec{Pkg: []string{"foo"}, Version: "1.0.0"}, []string{}, true},
+		{"vind stream error", []string{"foo"}, "1.0.0", &dummyInstallClient{recvErr: true}, &vin.InstallSpec{Pkg: []string{"foo"}, Version: "1.0.0"}, []string{}, true},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			c := client{c: test.client}
diff --git a/manifest.go b/manifest.go
index 8a9544a..43b2cf5 100644
--- a/manifest.go
+++ b/manifest.go
@@ -26,6 +26,10 @@ const (
 	// DefaultInstall is the command used to configure packages
 	// where a configure command is not provided
 	DefaultInstall = "make install {{ .MakeOpts }}"
+
+	// MetaManifestName is the name of the dummy manifest we use
+	// when installing multiple package at once
+	MetaManifestName = "packages"
 )
 
 // Dep represents a dependency tuple.
diff --git a/manifest_db.go b/manifest_db.go
index 3132964..9359cce 100644
--- a/manifest_db.go
+++ b/manifest_db.go
@@ -122,7 +122,7 @@ func (d *ManifestDB) loadManifests() (err error) {
 	}
 
 	for _, manifest := range manifests {
-		err = tx.Insert("package", manifest)
+		err = d.addManifest(tx, manifest)
 		if err != nil {
 			return
 		}
@@ -132,3 +132,19 @@ func (d *ManifestDB) loadManifests() (err error) {
 
 	return
 }
+
+func (d *ManifestDB) addManifest(tx *memdb.Txn, manifest *Manifest) error {
+	return tx.Insert("package", manifest)
+}
+
+func (d *ManifestDB) deleteManifest(name string) (err error) {
+	m, err := d.Latest(name, latest)
+	if err != nil {
+		return
+	}
+
+	tx := d.db.Txn(true)
+	defer tx.Commit()
+
+	return tx.Delete("package", m)
+}
diff --git a/proto/install.proto b/proto/install.proto
index 04032ad..0140105 100644
--- a/proto/install.proto
+++ b/proto/install.proto
@@ -9,7 +9,7 @@ package server;
 //
 // InstallSpec messages are sent via `vin install package [-v 1.0.0]`
 message InstallSpec {
-  string pkg = 1;
+  repeated string pkg = 1;
   string version = 2;
   bool force = 3;
 }
diff --git a/server.go b/server.go
index 7081ab7..cd4950a 100644
--- a/server.go
+++ b/server.go
@@ -55,8 +55,38 @@ func (s *Server) getOpsLock(oc chan string) {
 }
 
 func (s Server) Install(is *server.InstallSpec, vs server.Vin_InstallServer) (err error) {
-	if is.Pkg == "" {
+	var (
+		pkg string
+		ver version.Constraints
+	)
+
+	switch len(is.Pkg) {
+	case 0:
 		return fmt.Errorf("package must not be empty")
+
+	case 1:
+		pkg = is.Pkg[0]
+		if pkg == "" {
+			return fmt.Errorf("package must not be empty")
+		}
+
+		if is.Version != "" {
+			ver, err = version.NewConstraint(is.Version)
+			if err != nil {
+				return
+			}
+		}
+
+	default:
+		err = s.createMetaPackage(is.Pkg)
+		if err != nil {
+			return err
+		}
+
+		pkg = MetaManifestName
+		ver = latest
+
+		defer s.mdb.deleteManifest(pkg)
 	}
 
 	output := NewOutputter(vs)
@@ -65,22 +95,13 @@ func (s Server) Install(is *server.InstallSpec, vs server.Vin_InstallServer) (er
 	defer close(output.C)
 	go output.Dispatch()
 
-	output.C <- fmt.Sprintf("installing %s", is.Pkg)
+	output.C <- fmt.Sprintf("installing %s", pkg)
 
 	s.getOpsLock(output.C)
 	defer s.operationLock.Unlock()
 
-	var ver version.Constraints
-
-	if is.Version != "" {
-		ver, err = version.NewConstraint(is.Version)
-		if err != nil {
-			return
-		}
-	}
-
 	g := NewGraph(&s.mdb, &s.sdb, output.C)
-	tasks, err := g.Solve(DefaultProfile, is.Pkg, ver)
+	tasks, err := g.Solve(DefaultProfile, pkg, ver)
 	if err != nil {
 		return
 	}
@@ -163,7 +184,9 @@ func (s Server) Install(is *server.InstallSpec, vs server.Vin_InstallServer) (er
 		}
 	}
 
-	s.sdb.AddWorld(is.Pkg, is.Version)
+	if pkg != MetaManifestName {
+		s.sdb.AddWorld(pkg, is.Version)
+	}
 
 	return
 }
@@ -196,6 +219,37 @@ func (s Server) Version(ctx context.Context, _ *emptypb.Empty) (v *server.Versio
 	}, nil
 }
 
+func (s Server) createMetaPackage(packages []string) (err error) {
+	// create a new 'meta package'
+	deps := make([]Dep, len(packages))
+	for i, p := range packages {
+		if p == "" {
+			return fmt.Errorf("package must not be empty")
+		}
+
+		deps[i] = [2]string{p, ">=0"}
+	}
+
+	metaManifest := &Manifest{
+		Provides: MetaManifestName,
+		Version:  new(version.Version),
+		Meta:     true,
+		Profiles: map[string]Profile{
+			"default": Profile{
+				Deps: deps,
+			},
+		},
+	}
+
+	metaManifest.ID = metaManifest.String()
+
+	// add metaManifest to database
+	tx := s.mdb.db.Txn(true)
+	defer tx.Commit()
+
+	return s.mdb.addManifest(tx, metaManifest)
+}
+
 func installingLine(tasks []*Manifest) string {
 	sb := strings.Builder{}
 
diff --git a/server/install.pb.go b/server/install.pb.go
index 145c536..1be72f2 100644
--- a/server/install.pb.go
+++ b/server/install.pb.go
@@ -29,9 +29,9 @@ type InstallSpec struct {
 	sizeCache     protoimpl.SizeCache
 	unknownFields protoimpl.UnknownFields
 
-	Pkg     string `protobuf:"bytes,1,opt,name=pkg,proto3" json:"pkg,omitempty"`
-	Version string `protobuf:"bytes,2,opt,name=version,proto3" json:"version,omitempty"`
-	Force   bool   `protobuf:"varint,3,opt,name=force,proto3" json:"force,omitempty"`
+	Pkg     []string `protobuf:"bytes,1,rep,name=pkg,proto3" json:"pkg,omitempty"`
+	Version string   `protobuf:"bytes,2,opt,name=version,proto3" json:"version,omitempty"`
+	Force   bool     `protobuf:"varint,3,opt,name=force,proto3" json:"force,omitempty"`
 }
 
 func (x *InstallSpec) Reset() {
@@ -66,11 +66,11 @@ func (*InstallSpec) Descriptor() ([]byte, []int) {
 	return file_install_proto_rawDescGZIP(), []int{0}
 }
 
-func (x *InstallSpec) GetPkg() string {
+func (x *InstallSpec) GetPkg() []string {
 	if x != nil {
 		return x.Pkg
 	}
-	return ""
+	return nil
 }
 
 func (x *InstallSpec) GetVersion() string {
@@ -93,7 +93,7 @@ var file_install_proto_rawDesc = []byte{
 	0x0a, 0x0d, 0x69, 0x6e, 0x73, 0x74, 0x61, 0x6c, 0x6c, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x12,
 	0x06, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x22, 0x4f, 0x0a, 0x0b, 0x49, 0x6e, 0x73, 0x74, 0x61,
 	0x6c, 0x6c, 0x53, 0x70, 0x65, 0x63, 0x12, 0x10, 0x0a, 0x03, 0x70, 0x6b, 0x67, 0x18, 0x01, 0x20,
-	0x01, 0x28, 0x09, 0x52, 0x03, 0x70, 0x6b, 0x67, 0x12, 0x18, 0x0a, 0x07, 0x76, 0x65, 0x72, 0x73,
+	0x03, 0x28, 0x09, 0x52, 0x03, 0x70, 0x6b, 0x67, 0x12, 0x18, 0x0a, 0x07, 0x76, 0x65, 0x72, 0x73,
 	0x69, 0x6f, 0x6e, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x76, 0x65, 0x72, 0x73, 0x69,
 	0x6f, 0x6e, 0x12, 0x14, 0x0a, 0x05, 0x66, 0x6f, 0x72, 0x63, 0x65, 0x18, 0x03, 0x20, 0x01, 0x28,
 	0x08, 0x52, 0x05, 0x66, 0x6f, 0x72, 0x63, 0x65, 0x42, 0x23, 0x5a, 0x21, 0x67, 0x69, 0x74, 0x68,
diff --git a/server_test.go b/server_test.go
index 5034a01..75c1382 100644
--- a/server_test.go
+++ b/server_test.go
@@ -45,21 +45,24 @@ func TestServer_Install(t *testing.T) {
 
 	for _, test := range []struct {
 		name        string
-		pkg         string
+		pkg         []string
 		ver         string
 		expectError bool
 	}{
-		{"valid package, explicit version", "standalone", "1.0.0", false},
-		{"valid package, empty version", "standalone", "", false},
-		{"valid package, missing version", "standalone", "> 2.0.0", true},
-		{"invalid package", "foo", "", true},
-		{"valid package, invalid version", "standalone", "zzzzz", true},
-		{"valid package, bad checksum", "standalone", "0.1.1", true},
-		{"valid package, bad command template", "standalone", "0.1.2", true},
-		{"valid package, 404 archive", "standalone", "0.1.3", true},
-		{"erroring commands", "standalone", "0.1.0", true},
-		{"missing package", "", "", true},
-		{"meta package", "metaz", "", false},
+		{"valid package, explicit version", []string{"standalone"}, "1.0.0", false},
+		{"valid package, empty version", []string{"standalone"}, "", false},
+		{"valid package, missing version", []string{"standalone"}, "> 2.0.0", true},
+		{"invalid package", []string{"foo"}, "", true},
+		{"valid package, invalid version", []string{"standalone"}, "zzzzz", true},
+		{"valid package, bad checksum", []string{"standalone"}, "0.1.1", true},
+		{"valid package, bad command template", []string{"standalone"}, "0.1.2", true},
+		{"valid package, 404 archive", []string{"standalone"}, "0.1.3", true},
+		{"erroring commands", []string{"standalone"}, "0.1.0", true},
+		{"missing package", []string{}, "", true},
+		{"empty package", []string{""}, "", true},
+		{"multiple empty packages", []string{"", ""}, "", true},
+		{"multiple valid packages", []string{"standalone", "metaz"}, "", false},
+		{"meta package", []string{"metaz"}, "", false},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			// create an empty statedb
@@ -111,7 +114,7 @@ func TestServer_Install_WithService(t *testing.T) {
 	s.sdb, _ = LoadStateDB()
 
 	is := &server.InstallSpec{
-		Pkg:     "another-sample-app",
+		Pkg:     []string{"another-sample-app"},
 		Version: "",
 	}
 

From ec1feed3d734e33f76f39b4503506db4c9518878 Mon Sep 17 00:00:00 2001
From: jspc <james+git@zero-internet.org.uk>
Date: Fri, 11 Mar 2022 14:55:51 +0000
Subject: [PATCH 2/2] DRY-up empty package error

---
 server.go | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/server.go b/server.go
index cd4950a..9684c48 100644
--- a/server.go
+++ b/server.go
@@ -10,6 +10,8 @@ import (
 
 	"github.com/hashicorp/go-version"
 	"github.com/vinyl-linux/vin/server"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/types/known/emptypb"
 )
 
@@ -17,6 +19,10 @@ const (
 	DefaultProfile = "default"
 )
 
+var (
+	errEmptyPackage = status.Error(codes.InvalidArgument, "package must not be empty")
+)
+
 type Server struct {
 	server.UnimplementedVinServer
 
@@ -62,12 +68,12 @@ func (s Server) Install(is *server.InstallSpec, vs server.Vin_InstallServer) (er
 
 	switch len(is.Pkg) {
 	case 0:
-		return fmt.Errorf("package must not be empty")
+		return errEmptyPackage
 
 	case 1:
 		pkg = is.Pkg[0]
 		if pkg == "" {
-			return fmt.Errorf("package must not be empty")
+			return errEmptyPackage
 		}
 
 		if is.Version != "" {
@@ -224,7 +230,7 @@ func (s Server) createMetaPackage(packages []string) (err error) {
 	deps := make([]Dep, len(packages))
 	for i, p := range packages {
 		if p == "" {
-			return fmt.Errorf("package must not be empty")
+			return errEmptyPackage
 		}
 
 		deps[i] = [2]string{p, ">=0"}