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

Migrate dolt checkout to new CLI framework. #6209

Merged
merged 92 commits into from
Jul 11, 2023
Merged

Conversation

nicktobey
Copy link
Contributor

This PR migrates dolt branch to invoke SQL commands instead of manipulating the database directly. This allows it to work even on remote connections.

We implement this by adding an additional flag to the dolt_checkout stored proceudre: --global. This tells the SQL environment to persist this checkout for subsequent connections, making it the new default branch for future connections.

If a server is currently running, dolt_checkout("--global") will return an error. This is because we try to avoid changing the default branch on a server until we can properly consider the indented effect on existing connections.

Because working sets behave differently in our SQL environment than the command line (command line has a single working set, SQL land has a working set per-branch), we only permit changing the default branch when there would be no observable different in behavior: that is, when both the former and the new branch are clean. If this isn't the case, we return an error. This guarentees that a user calling dolt checkout from the command line will never see behavior that differs from the original CLI implementation. They will, at worst, get a helpful error message.

Copy link
Contributor

@macneale4 macneale4 left a comment

Choose a reason for hiding this comment

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

Looking good. A couple small style/flow suggestions. Ping me again when the existing and new bats tests are passing.

go/cmd/dolt/commands/checkout.go Outdated Show resolved Hide resolved
go/cmd/dolt/commands/checkout.go Outdated Show resolved Hide resolved
@nicktobey nicktobey force-pushed the nicktobey/dolt-checkout branch from 9ae8b72 to 1ce8051 Compare June 22, 2023 23:39
@nicktobey nicktobey force-pushed the nicktobey/dolt-checkout branch from 1ce8051 to 9ee4da1 Compare June 27, 2023 20:39
@nicktobey nicktobey force-pushed the nicktobey/dolt-checkout branch from 9ee4da1 to bb32bbb Compare June 27, 2023 21:07
@macneale4 macneale4 self-requested a review June 28, 2023 15:50
Copy link
Contributor

@macneale4 macneale4 left a comment

Choose a reason for hiding this comment

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

Main concern is that we need more tests for testing the state of the current branch when switching between remote and local mode. I'm not convinced that the current local-remote.bash approach covers this well enough.

go/cmd/dolt/commands/checkout.go Outdated Show resolved Hide resolved
go/cmd/dolt/commands/reset.go Outdated Show resolved Hide resolved
integration-tests/bats/sql-checkout.bats Outdated Show resolved Hide resolved
@nicktobey nicktobey force-pushed the nicktobey/dolt-checkout branch from a94789f to 9af1a71 Compare June 30, 2023 23:34
…local commands use the branch specified in repo_state.json as the default branch for the session.
@nicktobey nicktobey force-pushed the nicktobey/dolt-checkout branch from 3ee76db to ec7f29b Compare July 10, 2023 00:12
@nicktobey nicktobey force-pushed the nicktobey/dolt-checkout branch from 8ea54b7 to 5de290e Compare July 10, 2023 00:30
…hem once local commands use the branch specified in repo_state.json as the default branch for the session.
@nicktobey nicktobey force-pushed the nicktobey/dolt-checkout branch 2 times, most recently from 9d01739 to 4dfbce2 Compare July 10, 2023 04:12
@nicktobey nicktobey force-pushed the nicktobey/dolt-checkout branch from 99b73b6 to 9251a89 Compare July 10, 2023 06:57
integration-tests/bats/remotes.bats Outdated Show resolved Hide resolved
go/libraries/doltcore/sqle/dprocedures/init.go Outdated Show resolved Hide resolved
go/libraries/doltcore/sqle/dprocedures/dolt_checkout.go Outdated Show resolved Hide resolved
go/libraries/doltcore/sqle/dprocedures/dolt_checkout.go Outdated Show resolved Hide resolved
go/libraries/doltcore/sqle/dprocedures/dolt_checkout.go Outdated Show resolved Hide resolved
go/libraries/doltcore/sqle/dprocedures/dolt_checkout.go Outdated Show resolved Hide resolved
go/libraries/doltcore/sqle/dprocedures/dolt_checkout.go Outdated Show resolved Hide resolved
@macneale4 macneale4 self-requested a review July 10, 2023 23:27
Copy link
Contributor

@macneale4 macneale4 left a comment

Choose a reason for hiding this comment

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

Ship when the tests pass!

 .  o ..
 o . o o.o
      ...oo
        __[]__
     __|_o_o_o\__
     \""""""""""/
      \. ..  . /
 ^^^^^^^^^^^^^^^^^^^^

@nicktobey nicktobey merged commit 3eb8fc7 into main Jul 11, 2023
@nicktobey nicktobey deleted the nicktobey/dolt-checkout branch July 11, 2023 18:35
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.

6 participants