-
-
Notifications
You must be signed in to change notification settings - Fork 6k
Move git config/remote to gitrepo package and add global lock to resolve possible conflict when updating repository git config file #35151
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
Conversation
...lve possible conflict when updating repository git config file
No, it doesn't fix #32018
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't see why it is worth or right to do so, and don't see why it fixes that issue
Don't see why it is worth or right to do so, and don't see why it fixes that issue
git config and git remote write operations create a temporary file named config.lock. Since these operations are not atomic, they must not be run in parallel. If two requests attempt to modify the same repository concurrently—such as during a compare operation—one may fail due to the presence of an existing config.lock file.
In cases where config.lock is left behind due to an unexpected program exit, a global lock mechanism could allow us to safely remove the stale lock file when a related error is detected. While this behavior is not yet implemented in this PR, it is planned for a future enhancement.
So it is an improvement but doesn't "Fix #32018", right?
But does it really need the "global lock"? Is it possible that git already uses "*.lock" files by flock for global lock? Then Gitea shouldn't do anything more?
So it is an improvement but doesn't "Fix #32018", right?
But does it really need the "global lock"? Is it possible that git already uses "*.lock" files by
flockfor global lock? Then Gitea shouldn't do anything more?
The Git command-line interface is primarily designed for human interaction. When a git config write operation is initiated and a config.lock file is created, any subsequent git config write operation will immediately fail instead of waiting for the first one to complete.
While I couldn’t find official documentation confirming this behavior, the implementation can be found in the Git source code here:
https://github.com/git/git/blob/97e14d99f6def189b0f786ac6207b792ca3197b1/config.c#L3199
Based on this, it appears that Git does not implement a locking mechanism that causes subsequent operations to wait—it simply fails fast.
I believe this change partially addresses #32018, as there are three possible causes for the issue:
- Concurrent requests attempting to compare between the same base and forked repositories.
- A Git command exits unexpectedly, leaving behind a config.lock file. I plan to implement automatic cleanup of stale config.lock files in another PR, which would help resolve this case.
- An administrator manually runs Git commands on the Gitea server. This scenario is outside the scope of this PR, and I don’t believe it’s something we can reliably handle within the application.
Git already does a file-based "global lock"
(削除) https://github.com/git/git/blob/master/lockfile.c#L104 (config.c hold_lock_file_for_update -> lockfile.c lock_file_timeout ) (削除ここまで)
(削除) I do not think this PR is worth or right. (削除ここまで)
Update: git passes timeout=0, so it only locks "once"
...a into lunny/fix_git_config_conflict
Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
...a into lunny/fix_git_config_conflict
@lunny please fix the merge conflicts. 🍵
...a into lunny/fix_git_config_conflict
* main: [skip ci] Updated translations via Crowdin Use github.com/mholt/archives replace github.com/mholt/archiver (go-gitea#35390) Move some functions from package git to gitrepo (go-gitea#33910) Move git config/remote to gitrepo package and add global lock to resolve possible conflict when updating repository git config file (go-gitea#35151) Move HasWiki to repository service package (go-gitea#33912) Rename UpdateBranch API to RenameBranch API (go-gitea#35374)
* giteaofficial/main: [skip ci] Updated translations via Crowdin Use github.com/mholt/archives replace github.com/mholt/archiver (go-gitea#35390) Move some functions from package git to gitrepo (go-gitea#33910) Move git config/remote to gitrepo package and add global lock to resolve possible conflict when updating repository git config file (go-gitea#35151) Move HasWiki to repository service package (go-gitea#33912) Rename UpdateBranch API to RenameBranch API (go-gitea#35374)
Uh oh!
There was an error while loading. Please reload this page.
Partially fix #32018
git configandgit remotewrite operations create a temporary file namedconfig.lock. Since these operations are not atomic, they must not be run in parallel. If two requests attempt to modify the same repository concurrently—such as during a compare operation—one may fail due to the presence of an existingconfig.lockfile.In cases where
config.lockis left behind due to an unexpected program exit, a global lock mechanism could allow us to safely remove the stale lock file when a related error is detected. While this behavior is not yet implemented in this PR, it is planned for a future enhancement.