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

Use ServeProto::Serialise<ServeProto::BuildOptions> #1324

Conversation

Ericson2314
Copy link
Member

The interesting part is the preparatory commit:

Do not attempt to speak a newer version of the protocol

Both sides need to agree on a version (with std::min) for anything to work. Somehow... we've never done this.

With this comment, the next commit succeeds. Without this commit, the next commit fails. This is because the next commit exposes serializers which do different things for proto version 2.7, and we're currently requesting 2.6.

Opened NixOS/nix#9584 to track this issue

Both sides need to agree on a version (with `std::min`) for anything to
work. Somehow... we've never done this.

With this comment, the next commit succeeds. Without this commit, the
next commit fails. This is because the next commit exposes serializers
which do different things for proto version 2.7, and we're currently
requesting 2.6.

Opened NixOS/nix#9584 to track this issue
@Ericson2314
Copy link
Member Author

NixOS/nix#9584 is now idea-approved, which is the one question I had for this. The rest is entirely rote using serializers and changing no behavior, so I will merge this.

@Ericson2314 Ericson2314 merged commit f3a760a into NixOS:nix-next Dec 11, 2023
1 check passed
@Ericson2314 Ericson2314 deleted the serve-proto-build-options-serializer branch December 11, 2023 15:45
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.

1 participant