Is the order of fileDesc dependencies in generated code deterministic?

[haines]

Since upgrading to protobuf-es v2 we've been getting diffs in our generated code that look like

-  fileDesc("...", [file_buf_validate_validate, file_cerbos_request_v1_request, file_cerbos_response_v1_response, file_protoc_gen_openapiv2_options_annotations, file_google_api_field_behavior]);
+  fileDesc("...", [file_buf_validate_validate, file_cerbos_request_v1_request, file_cerbos_response_v1_response, file_google_api_field_behavior, file_protoc_gen_openapiv2_options_annotations]);

It seems to flip-flop between ordering file_protoc_gen_openapiv2_options_annotations and file_google_api_field_behavior first in the dependencies (and similarly higher up in the file, where they are imported).

Is it possible that these dependencies aren't deterministically ordered?

[timostamm]

The dependencies are provided by the compiler in the field repeated string dependency of message google.protobuf.FileDescriptorProto. A repeated field is ordered in Protobuf, and we should be maintaining the order through decoding, and also when we wrap the FileDescriptorProto into a DescFile and generate the call to fileDesc.

Can you verify that it's not the compiler introducing the changes? This should be as simple as dumping a descriptor set when you generate code (buf build -o or protoc -o), and inspecting it. If it isn't the compiler, it would be very helpful to have more data / information to reproduce. I've never seen unstable import order (we typically generate in CI and run a git diff, so unstable order should be noticed quickly), but it's possible that we don't have the exact set of files or environment.

[haines]

Thanks for the debugging tips. I ran buf build -o in a loop and sure enough I got a diff in the ordering of the dependency field after a few iterations.

Although it's not protobuf-es introducing the non-determinism, would it be possible for protobuf-es to sort the dependencies before generating the code so that the output is deterministic regardless?

[timostamm]

Hm, sorting in protoc-gen-es could have unintended side-effects, we'd have to think this through. At the very least, it will change the import order for many users...

I think it would be much preferable to get to the bottom of the issue. This could be a bug in buf. Do you mind sharing a reproducible example? Happy to talk via DM on slack if you don't want to share it here.

To unblock you, you could add this file:

// src/protoc-gen-es-sort-deps.cjs
const { runNodeJs } = require("@bufbuild/protoplugin");
const { protocGenEs } = require("@bufbuild/protoc-gen-es/dist/cjs/src/protoc-gen-es-plugin.js");

runNodeJs({
  name: "sort-deps",
  version: "v0",
  run(req) {
    req.protoFile.forEach(f => f.dependency.sort());
    return protocGenEs.run(req);
  }
});

It simply wraps protoc-gen-es and sorts dependencies. To use it, replace protoc-gen-es in buf.gen.yaml with:

version: v2
inputs:
  - directory: proto
plugins:
  - local: ["node", "src/protoc-gen-es-sort-deps.cjs"]
    opt: target=ts
    out: src/gen

[haines]

Nice one, thanks for the workaround.

I think this is probably a bug in buf. It seems to be related to type filtering (we have that set in our buf.gen.yaml, and the non-determinism only seems to appear if I use the --type option with buf build).

I've raised it as bufbuild/buf#3506, so I'll close this one.