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

Allow syncing to/from a local file #5543

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from
Open

Conversation

ChrisPenner
Copy link
Contributor

@ChrisPenner ChrisPenner commented Jan 14, 2025

One step towards syncv2 is using the syncv2 protocol for local file syncing. We can proceed from there :)

Overview

As a stepping stone to integrating SyncV2: adds sync.to-file, sync.from-file and sync.from-codebase commands.

The sync-file format will be identical to the stream emitted from Share (with possible exception of the framing boundary) so the majority of this code will be re-used then.

This may also possibly help with testing Unison code in CI, since it's much faster to sync.from-file than it is to pull from Share in CI, however even better than that would be to pull from Share quickly directly in CI, so of course work will continue on that.

Note that this will be slightly slower than it can be because I've left all the temp-entity checks in place, when/if the time comes that we can switch fully to syncv2 we can remove those and get an additional speedup.

Implementation notes

  • Adds new types for all the Stream Entity types
  • Adds copies of most errors and things since they differ slightly
  • Adds three new commands: sync.to-file, sync.from-file and sync.from-codebase

Interesting/controversial decisions

  • We've already discussed the stream format at length in meetings, so please let me know if you have any additional questions.

Test coverage

I've done manual testing of the new commands, but it would be good to start using them in CI as well, maybe we can ship a version of base using a Sync-file and use that to run base's unit tests against UCM builds in prod, that would've caught at least one runtime bug I shipped.

Also added property tests for serializing and deserializing stream formats.

Loose ends

  • This iteration of the format doesn't treat libs with any custom behaviour, and typically you don't need that when writing to file since you'll usually want to include all dependencies, but that'll need to be added for Share SyncV2
  • sync.from* is currently significantly slower than it needs to be, because all the queries which save to the codebase still check temp-entities and move dependents; which we don't need to do with this format. We can remove this slowdown once UCM is fully using syncV2 against share as well.

@ChrisPenner ChrisPenner changed the base branch from trunk to cp/project-ctx January 15, 2025 01:17
@ChrisPenner ChrisPenner changed the base branch from cp/project-ctx to trunk January 15, 2025 01:17
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This module contains the actual syncing logic.

@ChrisPenner ChrisPenner marked this pull request as ready for review January 17, 2025 19:14
@ChrisPenner ChrisPenner changed the title [WIP] Allow syncing to/from a local file Allow syncing to/from a local file Jan 17, 2025
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