Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non deterministic code gen #573

Closed
robin-anil opened this issue Dec 19, 2016 · 7 comments
Closed

Non deterministic code gen #573

robin-anil opened this issue Dec 19, 2016 · 7 comments
Labels

Comments

@robin-anil
Copy link
Contributor

robin-anil commented Dec 19, 2016

protobuf.js version: 6.2.1

Runpbjs -p <path-to-protobuf> -t json -o something.json
Output is not deterministic on every run.
Please ensure that the tree traversal and code generated is in a sorted and determinstic fashion.

This creates huge diffs on every checkin. This was not the case in v5.

I am upgrading a 50K line client side code base with over 100 protos from 5 to 6.

@dcodeIO
Copy link
Member

dcodeIO commented Dec 19, 2016

Unfortunately, I don't know where this could come from. Do you know if this affects specific parts / features of the generated output more than others?

@dcodeIO dcodeIO added the unsure label Dec 19, 2016
@robin-anil
Copy link
Contributor Author

I cannot discern where this is stemming from. I would think that a sorting would solve this issue.

@dcodeIO
Copy link
Member

dcodeIO commented Dec 19, 2016

Had an idea: pbjs is using async load, which most likely results in files being loaded in different order for large code bases. Replacing that with loadSync should prevent it.

@robin-anil
Copy link
Contributor Author

👍 Let me know when the fix is pushed. I will try it.

@paralin
Copy link

paralin commented Dec 19, 2016

@dcodeIO just add a --sync flag or something

dcodeIO added a commit that referenced this issue Dec 19, 2016
…ents to reflection namespaces, see #576; Also added Namespace#getEnum for completeness, see #576; Made pbjs use loadSync for deterministic outputs, see #573
@dcodeIO
Copy link
Member

dcodeIO commented Dec 19, 2016

Let me know if this fixes it!

@robin-anil
Copy link
Contributor Author

Verified to be fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants