protoc-gen-twirp generates non stable imports list

[rayjcwu]

If the protobuf definition has more than one imports, the generated code may change due to iterate over a map https://github.com/twitchtv/twirp/blob/master/protoc-gen-twirp/generator.go#L340-L342 .

This is an issue because we check in generated codes, and it keeps changing making our CI think someone forgot to check in the generated codes.

[marioizquierdo]

I think you refer to the order of imports being unreliable each time that the code is generated right?

That could be fixed by converting the map into a list and sorting it. What do you think?

[rayjcwu]

Thanks for the reply. Yes, non-stable order of imports is what I meant. I actually forked and patched using the same way as your suggestion

diff --git a/protoc-gen-twirp/generator.go b/protoc-gen-twirp/generator.go
index f331f65..4e42030 100644
--- a/protoc-gen-twirp/generator.go
+++ b/protoc-gen-twirp/generator.go
@@ -22,6 +22,7 @@ import (
        "go/printer"
        "go/token"
        "path"
+       "sort"
        "strconv"
        "strings"

@@ -337,8 +338,13 @@ func (t *twirp) generateImports(file *descriptor.FileDescriptorProto) {
                        }
                }
        }
-       for pkg, importPath := range deps {
-               t.P(`import `, pkg, ` `, importPath)
+       pkgs := make([]string, 0, len(deps))
+       for pkg := range deps {
+               pkgs = append(pkgs, pkg)
+       }
+       sort.Strings(pkgs)
+       for _, pkg := range pkgs {
+               t.P(`import `, pkg, ` `, deps[pkg])
        }
        if len(deps) > 0 {
                t.P()

, but I have some trouble to come up a test input for generateImports function, so I didn't send a pull request.

[rayjcwu]

@marioizquierdo I sent the PR, but I don't really know how to write the test case for that PR. Any example I could look at?

[rayjcwu]

Fixed by #312