Skip to content

Comments

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

Merged
Ericson2314 merged 2 commits intoNixOS:nix-nextfrom
obsidiansystems:serve-proto-build-options-serializer
Dec 11, 2023
Merged

Use ServeProto::Serialise<ServeProto::BuildOptions>#1324
Ericson2314 merged 2 commits intoNixOS:nix-nextfrom
obsidiansystems:serve-proto-build-options-serializer

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
@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