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.

Sign up for GitHub

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

Jump to bottom

commit with rebase-engine integration #7280

Merged
merged 8 commits into from
Feb 14, 2025
Merged

commit with rebase-engine integration #7280

merged 8 commits into from
Feb 14, 2025

Conversation

Byron
Copy link
Collaborator

@Byron Byron commented Feb 11, 2025 •
edited
Loading

This PR should make it possible to create a commit in various places in the commit-graph and automatically maintain the shape of the surrounding GitButler Workspace using the rebase engine.

Follow-up on #7271.

Tasks

  • refactor rebase-engine
  • support signing (and remove ref-handling from previous plumbing)
  • simple stack test and merge test
  • index handling - better validate that there is actually no changes after a commit operation.
    • tests with index visualization (to match respective tree)
  • do not use gitbutler-testsupport (pulls in everything) - port utils to but-testsupport
  • integrate rebase-engine into commit-engine
    • edit git references
    • edit virtual references
    • single-stack rebasing
    • test how to re-do a merge (and validate the tree)
    • test with upstreams (also per stack) not needed thanks to simplification via guided travresal
  • switch gix to main

For the next PR

  • figure out conflict handling (and prevent top-level merges to have conflicts (or existing merges for that matter))
  • port cherry_rebase_group to gix fully (key is the auto-conflict handling) - needed to do allow in-memory config adjustments
  • port merge-commits fully? (probably not needed )
  • update tauri integration to be able to create a complete commit at any position (reachable) in the graph.
  • insert-empty-commit as feature of but-rebase
    • move ref-rewriting there to support direct callers of but-rebase, re-order, etc.
  • tauri integration for amending commits
  • update gix to the version from main

Known Shortcomings (for now)

  • empty commits aren't specifically highlighted, or handled or prevented. The UI could detect that and show them.
  • Moving hunks or files will need multi-branch rebases with merge-commit handling. This can be added to the rebase engine as we know it today. This will also enable worktree-updates.
  • reordering in such a way that only heads a moved and nothing else happens (in terms of commit rewrites) probably wouldn't work yet in but-rebase.

Notes

  • Rebasing keeps existing headers like change-ids, but actually testing it is cumbersome but probably should be done nonetheless (needs to avoid 'stabilizing' change-ids).

For follow-up PRs

Remember: Frontend has 3 context-lines, and if there are any issues with this, we should try zero context lines

In any order (probably)

  • What happens in Git if one rebases non-linear branches? Can it retain the structure?
    • Rebase engine probably has to learn to re-schedule picks (and remember the base, a feature useful for multi-stacks as well, which then wouldn't need a special case anymore)
  • re-establish the worktree-lock in tauri for worktree-mutating operations.
    • read-only operations will probably be fine though, even though a read-lock would be required for correctness.
  • move file out of commit into worktree (uncommit something)
  • per-hunk exclusion if hunk didn't match (right now it rejects the whole file)
  • commit-signing tests
  • run hooks on an index created from the tree that would be committed.
  • move hunks from one commit into another
  • rebasing integration

Further Functionality (can be done with old API)

  • apply and unapply virtual branches

Out of scope

  • hunk-dependencies
    • These should be added once the commit-engine is feature-complete, the idea is that the UI can function well enough without them as a baseline.
  • atomic object writes
    • In theory, new objects should only be written to disk if they actually end up in a tree. For instance, if a change is rejected, the object associated with it shouldn't be in the object database.
    • However, even though implementing this with the in-memory object feature is very possible, it feels like an optimization for another day. In general, I really think only objects that end up in a tree should actually be written, so that the implementation doesn't have to rely on git gc to cleanup.

@vercel Vercel
Copy link

vercel bot commented Feb 11, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
gitbutler-components ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 14, 2025 1:32pm
gitbutler-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 14, 2025 1:32pm

@vercel vercel bot temporarily deployed to Preview – gitbutler-components February 11, 2025 16:06 Inactive
@vercel vercel bot temporarily deployed to Preview – gitbutler-web February 11, 2025 16:06 Inactive
@Byron Byron force-pushed the advanced-committing branch from 254b17b to 137ac82 Compare February 11, 2025 17:55
@github-actions github-actions bot added the rust Pull requests that update Rust code label Feb 11, 2025
@Byron Byron force-pushed the advanced-committing branch from 137ac82 to 2b4aafe Compare February 11, 2025 19:56
Byron added 3 commits February 11, 2025 21:02
@Byron
Small improvements for but-rebase whle taking a first look
45d35f6 
- remove unused dependencies
- isolate gix-repositories used in testing
- move error-handling tests into their own module to see where the
  actual work is done.
- refactor tests primarily to avoid `commits()` seeming too general.
- use a directory for tests to allow integration tests to grow into multiple
  modules, in a single test-binary.
- chain chainables for a more fluent look/better readability
- separate impl for utility/private methods to make pub-interface
  pop out when looking at source
- try to document the public API
- remove `Rebase` object in favor of an API similar to `OpenOptions::...::open()`,
  so the builder performs the one operation there is right away without intermediates.
  - this allowed for the bulk of it in a function.
- deduplicate validation
@Byron
Be specific about oid and relax some semantics
21b502c 
* Don't risk `refname` to be confused with a Git ref by renaming it.
* `oid` -> `commit_id`.
@Byron
move commit-creation (with signature support) into but-rebase
ba05d51 
There it can naturally be used by `but-workspace` and its
commit-creation.
@Byron Byron force-pushed the advanced-committing branch from 2b4aafe to cf517d9 Compare February 11, 2025 20:28
@Byron Byron force-pushed the advanced-committing branch from cf517d9 to 313feb9 Compare February 12, 2025 07:27
@Byron Byron force-pushed the advanced-committing branch from 313feb9 to 50ea5de Compare February 12, 2025 09:16
@Byron
Facilities to deal witht he first commit.
81bf8e8 
This relaxes the 'base' requirement and allows some operations
without a base.
@Byron
Use a more typesafe reference definition.
518936a 
This type can also be reused elsewhere to tie APIs together.
@Byron Byron force-pushed the advanced-committing branch from 2c69e0d to 2e72e91 Compare February 12, 2025 18:52
@Byron Byron force-pushed the advanced-committing branch from cd27f9c to 0ddcb02 Compare February 13, 2025 18:26
@Byron Byron force-pushed the advanced-committing branch from 0ddcb02 to 856e2b2 Compare February 13, 2025 19:11
@Byron Byron force-pushed the advanced-committing branch from 856e2b2 to 7e71089 Compare February 13, 2025 19:57
@Byron Byron force-pushed the advanced-committing branch from 7e71089 to d6c364c Compare February 14, 2025 08:10
@Byron Byron force-pushed the advanced-committing branch from d6c364c to 78ba985 Compare February 14, 2025 10:52
@Byron Byron force-pushed the advanced-committing branch from 78ba985 to 72d9569 Compare February 14, 2025 12:29
Byron added 2 commits February 14, 2025 14:30
@Byron
Update gix to the latest version
05bb75a
@Byron
Use but-rebase to rewrite the history up to tips of workspaces.
dc2b61d 
Additionally, provide the information needed to rewrite hidden/internal
references.
@Byron Byron force-pushed the advanced-committing branch from 72d9569 to dc2b61d Compare February 14, 2025 13:30
@Byron Byron marked this pull request as ready for review February 14, 2025 13:33
@Byron Byron enabled auto-merge February 14, 2025 13:33
@Byron Byron merged commit 2318c99 into master Feb 14, 2025
23 checks passed
@Byron Byron deleted the advanced-committing branch February 14, 2025 13:42
@Byron Byron mentioned this pull request Feb 14, 2025
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
rust Pull requests that update Rust code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@Byron