Change result type to array of refs

[hinshun]

Partially fixes #871

It moves non-array ref and refs map to array types while maintaining backwards compatibility. We need to detect clients without array ref capability (server need to detect client cap so apicap can't be used here), so AllowResultArrayRef was added.

Previous proto fields Result_Ref and Result_RefMap was moved to Result_RefDeprecated and Result_RefMapDeprecated respectively.

Some open questions:

1. Should we change the following struct/interfaces in this PR too?
   

   buildkit/frontend/result.go

   Lines 5 to 9 in 5c9365b

   	type Result struct {
   	Ref solver.CachedResult
   	Refs map[string]solver.CachedResult
   	Metadata map[string][]byte
   	}

   
   

   buildkit/frontend/gateway/client/result.go

   Lines 12 to 17 in 5c9365b

   	type Result struct {
   	mu sync.Mutex
   	Ref Reference
   	Refs map[string]Reference
   	Metadata map[string][]byte
   	}

2. In order to implement Previous solve results should be referable as locals in gw API #751 after this PR, we need to add edge digests for each ref ID. Are you envisioning something like this?

message Ref {
	repeated string ids = 1;
	repeated string edgeDigests = 2;
}

3. Where should we write tests?

[tonistiigi]

Should we change the following struct/interfaces in this PR too?

I think it makes sense do to this when we can update the exporters as well. It may even be needed to pass the ability to receive arrays based on exporter. All current builtin exporters should support it but current moby exporter would be problematic (that being said there is a branch where moby exporter doesn't exist any more).

If we don't add all that atm, then we should make sure to error whenever array with more than one item is returned. Also, we wouldn't add api cap now but when it is actually possible to return multiple items.

envisioning something like this?

Ideally I thought repeated Ref and no repeated id inside the Ref but I guess that would mean yet another type for the map, so not that important.

Where should we write tests?

Tests would come when it is possible to return multiple items. Atm just needs to keep everything working. CI is not passing atm, btw

[GordonTheTurtle]

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "ref-array" git@github.com:hinshun/buildkit.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842353889080
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

[tonistiigi]

@hinshun Is this ready? I see the erroring on multi-result array is not implemented. This is actually a bit problematic because if we pass AllowResultArrayRef now, but can't really handle multiple results then once frontends start returning them, old clients would ignore the result. So probably atm nothing is allowed to actually return multiple results and the client would need to pass another allowMultiArray parameter or a cap once it can actually handle them.

[hinshun]

@tonistiigi This isn't ready yet because I wanted to explore how #751 would be implemented first. In this #751 (comment), you mentioned:

Edge is digest + index, need it to implement https://godoc.org/github.com/moby/buildkit/client/llb#Output ToInput

I think that means we need both edge digests and edge indices right?

message Ref {
	repeated string ids = 1;
	repeated string edgeDigests = 2;
        repeated string edgeIndices = 3;
}

Alternatively, we should just add yet another type for the RefMap so it looks like this:

message Result {
	oneof result {
    		// Deprecated non-array refs.
		string refDeprecated = 1;
		RefMapDeprecated refsDeprecated = 2;

		RefArray refArray = 3;
		RefMap refMap = 4;
	}
	map<string, bytes> metadata = 10;
}

message RefMapDeprecated {
	map<string, string> refs = 1;
}

message RefArray {
    repeated Ref refs = 1;
}

message Ref {
    string id = 1;
    string digest = 2;
    string index = 3;
}

message RefMap {
	map<string, RefArray> refArraysByID = 1;
}

[hinshun]

I'm assuming this is where the error checking for multi-result array to be? This is where the forwarder is handling the Return RPC from the frontend.

[hinshun]

Originally, I wanted to just change the types to []string but only handle single element, but I think we should change these to support multi-result arrays right? i.e.

res.SetRef(&reference{id: ids, c: c})

[hinshun]

Same as above.

[tonistiigi]

@hinshun As mentioned in #751 you need the full definition for that. No way around that.

message Ref {
	repeated string ids = 1;
	repeated pb.Definition Definition = 2;
}

You can find the edge digest, and index from that data.

[hinshun]

I see, I misunderstood what you meant! That makes sense now.