Skip to content

Conversation

maoyama
Copy link
Owner

@maoyama maoyama commented May 27, 2025

Fixed an issue where a git error occurred when the app's pull button was pressed and when the git fetch run in the background (for the badge button displayed on the pull button) was executed at the same time.
Similar: microsoft/vscode#47141

I wrapped the execution of git fetch and pull with Actor to prevent them from running at the same time.

Before
Screenshot 2025-05-27 at 22 22 02
After
Screenshot 2025-05-27 at 22 20 26

maoyama added 9 commits May 27, 2025 08:45
- Added a new submodule 'TestFixtures/SyntaxHighlight2' with URL 'git@github.com:maoyama/SyntaxHighlight.git'.
- Initialized the submodule pointing to the specific commit.
- Introduced `outputSync` and integrated it into existing output methods
- Improved the ability to run a process synchronously with arguments and inputs.
- Introduced a new actor `GitFetchExecutor` for single access handling of git operations.
- Replaced direct calls to `Process.output` for `GitFetch` and `GitPull` commands with calls to `GitFetchExecutor.shared.execute` for better structure and maintainability.
- Updated the project.pbxproj to include the new `GitFetchExecutor.swift` file.
@maoyama maoyama requested a review from Copilot May 27, 2025 13:35
Copy link
Contributor

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

A fix to serialize concurrent Git fetch and pull operations using an actor, preventing “cannot lock ref” errors by wrapping calls with a single executor.

  • Introduce GitFetchExecutor actor to serialize fetch/pull commands
  • Update views and sync logic to replace Process.output(...) calls with GitFetchExecutor.shared.execute(...)
  • Extend Process+Run with synchronous outputSync methods and generic overloads
  • Add a placeholder test file GitPullParallelTests.swift

Reviewed Changes

Copilot reviewed 6 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
GitClientTests/GitPullParallelTests.swift Added an empty test stub for parallel pull scenarios
GitClient/Views/Folder/FolderView.swift Swapped direct Process.output call for GitFetchExecutor.shared.execute(GitPull...)
GitClient/Views/BranchesView.swift Replaced Process.output(GitFetch...) with GitFetchExecutor.shared.execute(GitFetch...)
GitClient/Models/Observables/SyncState.swift Updated background fetch to use the executor instead of Process.output
GitClient/Models/Commands/GitFetchExecutor.swift New actor GitFetchExecutor with synchronous execute methods
GitClient/Extensions/Process+Run.swift Added outputSync helpers, generic overloads, and updated logging
Files not reviewed (3)
  • .gitmodules: Language not supported
  • GitClient.xcodeproj/project.pbxproj: Language not supported
  • TestFixtures/SyntaxHighlight2: Language not supported
Comments suppressed due to low confidence (3)

GitClientTests/GitPullParallelTests.swift:12

  • The test method is empty; add assertions or a meaningful implementation to validate parallel pull behavior.
@Test func gitPull() async throws {

GitClient/Extensions/Process+Run.swift:37

  • This async function declares a return type but does not return the result. Add return before try outputSync(...).
        try outputSync(arguments: arguments, currentDirectoryURL: currentDirectoryURL, inputs: inputs)

GitClient/Extensions/Process+Run.swift:82

  • The generic async output<G: Git> method must return the parsed model. Add return try outputSync(git) instead of just calling it.
        try outputSync(git)

actor GitFetchExecutor {
static let shared = GitFetchExecutor()

func execute(_ command: GitFetch) throws {
Copy link
Preview

Copilot AI May 27, 2025

Choose a reason for hiding this comment

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

Actor-isolated methods should be marked async to allow await GitFetchExecutor.shared.execute(...) calls. Change signature to func execute(_ command: GitFetch) async throws.

Copilot uses AI. Check for mistakes.

@maoyama maoyama merged commit 62321a2 into main May 27, 2025
4 checks passed
@maoyama maoyama deleted the fix-pulling-error branch May 31, 2025 08:49
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