Skip to content

Use gix in operating_mode and avoid panic#12335

Open
krlvi wants to merge 1 commit intomasterfrom
codex/improve-operating_mode-function-to-use-gix
Open

Use gix in operating_mode and avoid panic#12335
krlvi wants to merge 1 commit intomasterfrom
codex/improve-operating_mode-function-to-use-gix

Conversation

@krlvi
Copy link
Member

@krlvi krlvi commented Feb 11, 2026

Motivation

  • Avoid panics from unwrapping the legacy git2 repository.
  • Unify HEAD inspection to gix so operating_mode can fail gracefully.

Description

  • gitbutler_operating_modes::operating_mode now uses ctx.repo.get() (gix) instead of ctx.git2_repo.get().unwrap().
  • All failure paths return OperatingMode::OutsideWorkspace (no panics). If the repo handle can't be opened/borrowed, we return OutsideWorkspaceMetadata::default() directly.
  • HEAD/ref inspection uses gix (head().referent_name()) and compares against OPEN_WORKSPACE_REFS / EDIT_BRANCH_REF via byte-wise comparisons.
  • Minor cleanup: factor repeated OutsideWorkspace construction into a local helper and avoid redundant metadata computation when repo access already failed.

Files:

  • crates/gitbutler-operating-modes/src/lib.rs

Regression coverage added:

  • Fallback when the repo cannot be opened (.git removed).
  • Fallback when on gitbutler/edit without edit-mode metadata.

Copilot AI review requested due to automatic review settings February 11, 2026 22:36
@vercel
Copy link

vercel bot commented Feb 11, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
gitbutler-web Ignored Ignored Preview Feb 16, 2026 5:46pm

Request Review

@github-actions github-actions bot added the rust Pull requests that update Rust code label Feb 11, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates gitbutler_operating_modes::operating_mode to use the gix repository from but_ctx::Context instead of unwrapping the legacy git2 repo, preventing panics and allowing the function to fail gracefully.

Changes:

  • Replaced ctx.git2_repo.get().unwrap() with ctx.repo.get() and added graceful fallback to OperatingMode::OutsideWorkspace on failure.
  • Switched HEAD reference name inspection/comparisons to gix (referent_name() + byte-wise comparisons) for workspace/edit branch detection.

@krlvi krlvi force-pushed the codex/improve-operating_mode-function-to-use-gix branch 2 times, most recently from cbe5415 to 14ab770 Compare February 14, 2026 07:49
Copilot AI review requested due to automatic review settings February 14, 2026 07:55
@krlvi krlvi force-pushed the codex/improve-operating_mode-function-to-use-gix branch from 14ab770 to ac77abb Compare February 14, 2026 07:55
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@krlvi krlvi force-pushed the codex/improve-operating_mode-function-to-use-gix branch from ac77abb to 6f1e271 Compare February 14, 2026 08:01
@krlvi krlvi requested a review from Copilot February 14, 2026 21:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@krlvi krlvi requested a review from Caleb-T-Owens February 14, 2026 21:41
@Caleb-T-Owens
Copy link
Contributor

@krlvi I just want to make sure I understand the problem we're solving here.

It looks like the function may be getting called in cases where there isn’t a git repository. If that’s the case, I’m not sure returning OperatingMode::OutsideWorkspace makes the most sense.

Also, with hindsight, unwrapping here seems like a bad idea, so we should probably refactor it to be a Result type.

Would it make sense to update the return type to Result<Option<OperatingMode>>, where Ok(None) represents the absence of a repository? Or would it be cleaner to introduce a NoRepository operating mode instead?

WDYT?

@krlvi
Copy link
Member Author

krlvi commented Feb 14, 2026

@krlvi I just want to make sure I understand the problem we're solving here.

It looks like the function may be getting called in cases where there isn’t a git repository. If that’s the case, I’m not sure returning OperatingMode::OutsideWorkspace makes the most sense.

Also, with hindsight, unwrapping here seems like a bad idea, so we should probably refactor it to be a Result type.

Would it make sense to update the return type to Result<Option<OperatingMode>>, where Ok(None) represents the absence of a repository? Or would it be cleaner to introduce a NoRepository operating mode instead?

WDYT?

My objectives were:

  • remove the panic
  • remove the usage of git2

Happy to change the api, but im wondering if we want to go from OperatingMode to Result<Option<OperatingMode>> 🤷‍♂️
Maybe just a Option<OperatingMode> to represent not having a repo?

But I am also thinking, since "operating mode" is generic enough, maybe there could be a variant for "not a git repo" ?

@Caleb-T-Owens
Copy link
Contributor

@krlvi I just want to make sure I understand the problem we're solving here.
It looks like the function may be getting called in cases where there isn’t a git repository. If that’s the case, I’m not sure returning OperatingMode::OutsideWorkspace makes the most sense.
Also, with hindsight, unwrapping here seems like a bad idea, so we should probably refactor it to be a Result type.
Would it make sense to update the return type to Result<Option<OperatingMode>>, where Ok(None) represents the absence of a repository? Or would it be cleaner to introduce a NoRepository operating mode instead?
WDYT?

My objectives were:

  • remove the panic
  • remove the usage of git2

Happy to change the api, but im wondering if we want to go from OperatingMode to Result<Option<OperatingMode>> 🤷‍♂️ Maybe just a Option<OperatingMode> to represent not having a repo?

But I am also thinking, since "operating mode" is generic enough, maybe there could be a variant for "not a git repo" ?

I think it would be helpful to refactor this to return a result while we're looking at it. I'm happy to take this on if you'd like.

After giving it some thought, I like the idea of using an OperatingMode variant to represent a missing repo.

@krlvi
Copy link
Member Author

krlvi commented Feb 15, 2026

@krlvi I just want to make sure I understand the problem we're solving here.
It looks like the function may be getting called in cases where there isn’t a git repository. If that’s the case, I’m not sure returning OperatingMode::OutsideWorkspace makes the most sense.
Also, with hindsight, unwrapping here seems like a bad idea, so we should probably refactor it to be a Result type.
Would it make sense to update the return type to Result<Option<OperatingMode>>, where Ok(None) represents the absence of a repository? Or would it be cleaner to introduce a NoRepository operating mode instead?
WDYT?

My objectives were:

  • remove the panic
  • remove the usage of git2

Happy to change the api, but im wondering if we want to go from OperatingMode to Result<Option<OperatingMode>> 🤷‍♂️ Maybe just a Option<OperatingMode> to represent not having a repo?
But I am also thinking, since "operating mode" is generic enough, maybe there could be a variant for "not a git repo" ?

I think it would be helpful to refactor this to return a result while we're looking at it. I'm happy to take this on if you'd like.

After giving it some thought, I like the idea of using an OperatingMode variant to represent a missing repo.

Okay feel free to take over this thing but only if you have the bandwidth for it (i.e. it's not interfering with the rebase engine stuff). If you wanna take it, just close this PR

@krlvi
Copy link
Member Author

krlvi commented Feb 16, 2026

ping @Caleb-T-Owens how should we do here?

@krlvi krlvi force-pushed the codex/improve-operating_mode-function-to-use-gix branch from 6f1e271 to 1b8ed02 Compare February 16, 2026 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants