pokt-network / poktroll

The official Shannon upgrade implementation of the Pocket Network Protocol implemented using the Cosmos SDK
MIT License
16 stars 8 forks source link

[Code Health] Investigate a permanent solution to pulsar self-import issue #405

Open bryanchriswhite opened 8 months ago

bryanchriswhite commented 8 months ago

Objective

Remove proto_fix_self_import make target workaround.

Origin Document

  1. Post v0.50.x cosmos-sdk leverages the v2 go protobuf API, which necessitates tracking api/poktroll/**/*.go (generated) files in version control. Otherwise, the ignite CLI becomes unusable as it requries go mod tidy to succeed for all subcommands, which is not possible without these files as they are imported by each respective module's module.go and autocli.go files.
  2. When .proto files import protobuf types from other .proto files, it seems that we MUST add a corresponding option in the plugins.opt field in buf.gen.pulsar.yaml to mitigate the following error during generation: image
  3. So long as we MUST do 2 (and we do not apply the workaround), a new self-import error arises in the corresponding generated *.pulsar.go code immediately after generation (i.e. try ignite generate proto-go && ignite chain build --skip-proto): image

Goals

Deliverables

Non-goals / Non-deliverables

General deliverables


Creator: @bryanchriswhite Co-Owners: @red-0ne

bryanchriswhite commented 2 months ago

Try renaming any .proto files which are currently named after their respective modules; e.g., proto/poktroll/session/session.proto, proto/poktroll/service/service.proto, etc.

See:

image

We can also probably remove these files from the [buf.gen.pulsar.yaml]() plugins.opt field:

plugins:
  - name: go-pulsar
    out: ./api
-    opt: paths=source_relative,Mpoktroll/shared/service.proto=github.com/pokt-network/poktroll/api/poktroll/shared,Mpoktroll/shared/supplier.proto=github.com/pokt-network/poktroll/api/poktroll/shared,Mpoktroll/supplier/supplier.proto=github.com/pokt-network/poktroll/api/poktroll/supplier,Mpoktroll/session/session.proto=github.com/pokt-network/poktroll/api/poktroll/session
+    opt: paths=source_relative
bryanchriswhite commented 2 months ago

Try renaming any .proto files which are currently named after their respective modules; e.g., proto/poktroll/session/session.proto, proto/poktroll/service/service.proto, etc.

We can also probably remove these files from the buf.gen.pulsar.yaml plugins.opt field:

No dice. :game_die: The proto_fix_self_import target seems to still be necessary despite these changes (see: #749). :disappointed:

Olshansk commented 1 week ago

@bryanchriswhite Is this still an open issue? Please close out if not.

Olshansk commented 1 week ago

@bryanchriswhite Is this still an open issue? Please close out if not.

bryanchriswhite commented 1 week ago

This is still an issue that we're applying a workaround for.

Olshansk commented 1 week ago

@bryanchriswhite Did you ever open an issue in https://github.com/cosmos/cosmos-proto?

Seemed to be one of the notes in the original description.

bryanchriswhite commented 1 week ago

I did not. I wanted to ensure that it wasn't our fault first. My plan was to scaffold a new chain and try to reproduce the issue there. I think this exercise will also improve the quality of any issue that we do open.