Skip to content

Conversation

@boukeversteegh
Copy link
Collaborator

@boukeversteegh boukeversteegh commented Jul 4, 2020

Protobuf itself does not allow circular dependencies (import "a.proto" in b.proto, and import "b.proto" in a.proto).

So why do we still get circular import errors in python? Because protobuf dependencies are on file-level, while betterproto dependencies are on package-level.

Example:

The proto messages depend on each other in a non-circular way:

Test -------> RootPackageMessage <--------------.
  `------------------------------------> OtherPackageMessage

Test and RootPackageMessage are in different files, but belong to the same package (root):

(Test -------> RootPackageMessage) <------------.
  `------------------------------------> OtherPackageMessage

After grouping the packages into single files or modules, a circular dependency is created:

(root: Test & RootPackageMessage) <-------> (other: OtherPackageMessage)

Because we don't know in advance which imports will cause a circular-dependency (as we currently do single-pass compiling), the only solution I see is to always use forward references ("package.SomeType"), and have the import statements at the bottom.

This PR implements that solution.

@boukeversteegh boukeversteegh changed the title Fix/circular dependencies Import bug - Circular Dependencies Jul 4, 2020
@boukeversteegh boukeversteegh force-pushed the fix/circular-dependencies branch from 0af0cf4 to 23dcbc2 Compare July 4, 2020 13:53
@boukeversteegh boukeversteegh requested a review from nat-n July 4, 2020 13:54
@boukeversteegh boukeversteegh added this to the Better Imports milestone Jul 4, 2020
@boukeversteegh boukeversteegh added bug Something isn't working has test Has a (xfail) test that verifies the bugfix or feature labels Jul 4, 2020
@boukeversteegh boukeversteegh merged commit 3273ae4 into danielgtaylor:master Jul 7, 2020
@abn abn mentioned this pull request Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working has test Has a (xfail) test that verifies the bugfix or feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants