Skip to content

Bug fix allowing diamond dependencies.#8

Open
saarraz wants to merge 2 commits intobrianchirls:masterfrom
saarraz:patch-1
Open

Bug fix allowing diamond dependencies.#8
saarraz wants to merge 2 commits intobrianchirls:masterfrom
saarraz:patch-1

Conversation

@saarraz
Copy link
Copy Markdown

@saarraz saarraz commented Mar 25, 2016

The current version would fail if a "diamond" dependency existed, for example:
a.proto:
message A { }
b.proto:
import "a.proto";
message B { }
c.proto:
import "a.proto";
import "b.proto"; --> ERROR: Duplicate message A defined.

The previous code does have a mechanism to avoid this (the 'loaded' object) but it is not passed down to recursive calls into readProto, which is a bug.
Passing it down fixes the problem.

saarraz added 2 commits March 25, 2016 13:13
The current version would fail if a "diamond" dependency existed, for example:
a.proto:
    message A { }
b.proto:
    import "a.proto";
    message B { }
c.proto:
    import "a.proto";
    import "b.proto"; --> ERROR: Duplicate message A defined.

The previous code does have a mechanism to avoid this (the 'loaded' object) but it is not passed down to recursive calls into readProto, which is a bug.
Passing it down fixes the problem.
@sdvcrx
Copy link
Copy Markdown

sdvcrx commented Jun 10, 2016

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants