Skip to content

Refactor git command context & pipeline#36406

Merged
wxiaoguang merged 3 commits intogo-gitea:mainfrom
wxiaoguang:refactor-git-cmd
Jan 21, 2026
Merged

Refactor git command context & pipeline#36406
wxiaoguang merged 3 commits intogo-gitea:mainfrom
wxiaoguang:refactor-git-cmd

Conversation

@wxiaoguang
Copy link
Contributor

Less and simpler code, fewer bugs

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 19, 2026
@wxiaoguang wxiaoguang requested a review from Copilot January 19, 2026 02:49
@github-actions github-actions bot added modifies/go Pull requests that update Go code docs-update-needed The document needs to be updated synchronously labels Jan 19, 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 refactors the git command execution pipeline to simplify code and improve maintainability. The core changes replace manual os.Pipe() management with a cleaner WithStdoutReader approach, and introduce a new gitcmd.Context interface that provides CancelWithCause for better error propagation.

Changes:

  • Introduced new gitcmd.Context interface with CancelWithCause method for cleaner error handling in pipeline functions
  • Replaced manual os.Pipe() creation and cleanup with WithStdoutReader pattern across all git command usages
  • Removed unused git timeout configurations (Default, Clone, Pull) and simplified timeout handling logic
  • Updated process.Manager to use CancelCauseFunc instead of context.CancelFunc for better cause tracking

Reviewed changes

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

Show a summary per file
File Description
modules/git/gitcmd/context.go New file introducing the Context interface with CancelWithCause method
modules/git/gitcmd/command.go Removed default timeout logic, updated pipeline function signature to use new Context interface
modules/process/process.go Changed Cancel field type from context.CancelFunc to CancelCauseFunc
modules/process/manager.go Updated context creation to use WithCancelCause, introduced CancelCauseFunc type
modules/queue/workerqueue.go Updated ctxRunCancel type to process.FinishedFunc
modules/nosql/manager.go Updated finished field type to process.FinishedFunc
modules/log/init.go Updated to convert FinishedFunc to context.CancelFunc
modules/setting/git.go Removed Default, Clone, and Pull timeout configuration fields
custom/conf/app.example.ini Removed unused timeout configuration examples
modules/git/git.go Removed SetDefaultCommandExecutionTimeout call
services/cron/tasks_basic.go Changed to use Git.Timeout.GC instead of Default
services/repository/gitgraph/graph.go Replaced os.Pipe with WithStdoutReader, updated pipeline function signature
services/repository/files/temp_repo.go Replaced os.Pipe with WithStdoutReader, updated pipeline function signature
services/repository/contributors_graph.go Replaced os.Pipe with WithStdoutReader, removed manual close
services/pull/pull.go Replaced os.Pipe with WithStdoutReader, updated pipeline function signature
services/pull/patch_unmerged.go Replaced os.Pipe with WithStdoutReader, updated pipeline function signature
services/pull/patch.go Updated pipeline function signature, removed manual stderr close
services/pull/merge_prepare.go Replaced os.Pipe with WithStdoutReader, renamed filepath to treePath for clarity
services/gitdiff/gitdiff.go Removed explicit timeout setting
routers/private/hook_verification.go Replaced os.Pipe with WithStdoutReader for both verifyCommits and readAndVerifyCommit
modules/git/submodule.go Replaced os.Pipe with WithStdoutReader, removed manual close
modules/git/repo_stats.go Replaced os.Pipe with WithStdoutReader, removed manual close
modules/git/grep.go Updated pipeline function to use new Context interface with CancelWithCause
modules/git/diff.go Replaced os.Pipe with WithStdoutReader, updated pipeline function signature
modules/git/catfile_batch_reader.go Removed WithUseContextTimeout call
modules/gitrepo/blame.go Removed WithUseContextTimeout call
routers/web/repo/githttp.go Minor formatting improvement for multi-line function call

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Jan 19, 2026
@wxiaoguang wxiaoguang force-pushed the refactor-git-cmd branch 2 times, most recently from bf1d78b to 6e946ab Compare January 19, 2026 05:22
@wxiaoguang wxiaoguang added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Jan 19, 2026
@wxiaoguang wxiaoguang added this to the 1.26.0 milestone Jan 19, 2026
@GiteaBot GiteaBot removed the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 19, 2026
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jan 19, 2026
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 20, 2026
@wxiaoguang wxiaoguang enabled auto-merge (squash) January 21, 2026 01:30
@wxiaoguang wxiaoguang merged commit 9ea91e0 into go-gitea:main Jan 21, 2026
24 checks passed
@wxiaoguang wxiaoguang deleted the refactor-git-cmd branch January 21, 2026 01:48
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jan 21, 2026
* giteaofficial/main: (27 commits)
  Allow foreachref parse max tokens from 4*64KB to 4MB (go-gitea#36414)
  Refactor git command context & pipeline (go-gitea#36406)
  Fix missing repository id when migrating release attachments (go-gitea#36389)
  [skip ci] Updated translations via Crowdin
  Refactor git command stderr handling (go-gitea#36402)
  Some refactors about GetMergeBase (go-gitea#36186)
  Refactor git command stdio pipe (go-gitea#36393)
  fix: typos in comments (go-gitea#36394)
  add support for archive-upload rpc (go-gitea#36391)
  Hide delete directory button for mirror or archive repository and disable the menu item if user have no permission (go-gitea#36384)
  Fix CODEOWNERS review request attribution using comment metadata (go-gitea#36348)
  Update JS and PY deps (go-gitea#36383)
  Add ability to download subpath archive (go-gitea#36371)
  Fix bug on notification read (go-gitea#36339)
  Add chunked transfer encoding support for LFS uploads (go-gitea#36380)
  Migrate to `import.meta.env` and clean up types and eslint (go-gitea#36362)
  Rename CSS variables and improve colorblind themes (go-gitea#36353)
  Indicate when only optional checks failed (go-gitea#36367)
  Release attachments must belong to the intended repo (go-gitea#36347)
  Fix permission check on org project operations (go-gitea#36318)
  ...
@wxiaoguang
Copy link
Contributor Author

Found an incorrect test, also fixed in Refactor git command stdio pipe #36422 together

--- FAIL: TestRunWithContextTimeout (0.00s)
    --- FAIL: TestRunWithContextTimeout/WithTimeout (0.00s)
        command_test.go:125: 
            	Error Trace:	/home/runner/work/gitea/gitea/modules/git/gitcmd/command_test.go:125
            	Error:      	Expected error with "context deadline exceeded" in chain but got nil.
            	Test:       	TestRunWithContextTimeout/WithTimeout
FAIL

ZPascal added a commit to ZPascal/gitea that referenced this pull request Jan 22, 2026
# This is the 1st commit message:

feat: Add max-parallel implementation inside the Gitea server

# This is the commit message go-gitea#2:

fix: Remove MatrixID and Capacity functionality

# This is the commit message go-gitea#3:

Fix incorrect text content detection (go-gitea#36364)

Fix go-gitea#36325
# This is the commit message go-gitea#4:

clean watches when make a repository private and check permission when send release emails (go-gitea#36319)

# This is the commit message go-gitea#5:

Fix bug when compare in the pull request (go-gitea#36363)

The pull request comparison should not use `direct compare`.
# This is the commit message go-gitea#6:

Fix permission check on org project operations (go-gitea#36318)

# This is the commit message go-gitea#7:

Release attachments must belong to the intended repo (go-gitea#36347)

# This is the commit message go-gitea#8:

Indicate when only optional checks failed (go-gitea#36367)

Currently it's not clear that you can merge a PR when only optional
checks failed:

<img width="922" height="447" alt="Screenshot 2026-01-14 at 4 08 17 pm"
src="https://github.com/user-attachments/assets/e11670c7-5ab9-42d7-af09-2d8a8fd532d3"
/>

This PR changes the text to say "Some optional checks failed" when only
optional checks failed:

<img width="922" height="443" alt="Screenshot 2026-01-14 at 3 59 08 pm"
src="https://github.com/user-attachments/assets/9ea69b13-38d6-4cfc-b4f7-952eff58e546"
/>

When a required check fails it'll still say "Some checks failed":

<img width="928" height="343" alt="Screenshot 2026-01-14 at 3 59 20 pm"
src="https://github.com/user-attachments/assets/d3764a95-9737-4482-851e-d3406b1e4d76"
/>

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
# This is the commit message go-gitea#9:

Rename CSS variables and improve colorblind themes (go-gitea#36353)

Followup go-gitea#36215, rename the
variables for consistency with existing vars and change green to value
of `--color-blue` in the relevant color blind themes:

<img width="1305" height="303" alt="image"
src="https://github.com/user-attachments/assets/3d131ab7-99ab-4b03-93ab-715ce0030b08"
/>

The blue coloring also matched GitHub:

<img width="1313" height="393" alt="image"
src="https://github.com/user-attachments/assets/f97e35b2-4ff4-49b0-841f-ffd49a02e03d"
/>

---------

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
# This is the commit message go-gitea#10:

Migrate to `import.meta.env` and clean up types and eslint (go-gitea#36362)

`import.meta.env` is supported in both vitest and webpack [as of
recent](webpack/webpack#19996), so replace all
previous use of `process.env` with it. Current usage is limited to test
files, I've also verified it works in actual frontend code.

`webpack/module` is added to typescript types which includes the
definition for `import.meta.env`. I've also made the eslint globals more
precise. Finally, `__webpack_public_path__` is removed from our type
definitions because `webpack/module` also provides it.
# This is the commit message go-gitea#11:

Add chunked transfer encoding support for LFS uploads (go-gitea#36380)

Enable chunked transfer encoding for Git LFS uploads by adding
Transfer-Encoding: chunked header to upload action responses. This
prevents large file uploads (100+ MB) from being blocked by reverse
proxies like Cloudflare that buffer non-chunked requests.

Fix go-gitea#22233

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
# This is the commit message go-gitea#12:

Fix bug on notification read (go-gitea#36339)

When a user has been revoked permission to access a repository, the
related notification could still be visited. But the repository's
information should not be leaked any more.
# This is the commit message go-gitea#13:

Add ability to download subpath archive (go-gitea#36371)

closes: go-gitea#4478

---------

Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
# This is the commit message go-gitea#14:

Update JS and PY deps (go-gitea#36383)

- Update JS and PY dependencies
- Workaround stylelint/stylelint#8893 by
moving the stylint config file to JS
- Regenerate SVGs
- Bump to python 3.14 in devcontainer and actions
- Verified `@github/text-expander-element`
- Removed obsolete type stub
# This is the commit message go-gitea#15:

Fix CODEOWNERS review request attribution using comment metadata (go-gitea#36348)

Fixes go-gitea#36333

## Problem

When CODEOWNERS automatically assigns reviewers to a pull request, the
timeline incorrectly shows the PR author as the one who requested the
review (e.g., "PR_AUTHOR requested review from CODE_OWNER"). This is
misleading since the action was triggered automatically by CODEOWNERS
rules, not by the PR author.

## Solution

Store CODEOWNERS attribution in comment metadata instead of changing the
doer user:
- Add `SpecialDoerName` field to `CommentMetaData` struct (value:
`"CODEOWNERS"` for CODEOWNERS-triggered requests)
- Pass `isCodeOwners=true` to `AddReviewRequest` and
`AddTeamReviewRequest` functions
- Template can check this metadata to show appropriate attribution
message

---------

Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
# This is the commit message go-gitea#16:

Hide delete directory button for mirror or archive repository and disable the menu item if user have no permission (go-gitea#36384)

# This is the commit message go-gitea#17:

add support for archive-upload rpc (go-gitea#36391)

Add support for fetching archives with `git archive --remote <repo-url>`

closes: go-gitea#23425

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

# This is the commit message go-gitea#18:

fix: typos in comments (go-gitea#36394)

# This is the commit message go-gitea#19:

Refactor git command stdio pipe (go-gitea#36393)

And remove the incorrect `ensureValidGitRepository`
# This is the commit message go-gitea#20:

Some refactors about GetMergeBase (go-gitea#36186)

Maybe fix go-gitea#32018

- Use `gitrepo.GetMergeBase` method instead of other two
implementations.
- Add `FetchRemoteCommit` so that we don't need to add many `remote` to
the git repository to avoid possible git lock conflicts. A lock will
start when invoke the function, it will be invoked when cross-repository
comparing. The head repository will fetch the base repository's base
commit id. In most situations, it should lock the fork repositories so
that it should not become a bottleneck.
- Improve `GetCompareInfo` to remove unnecessarily adding remote.
- Remove unnecessary parameters of `SignMerge`.
# This is the commit message go-gitea#21:

Refactor git command stderr handling (go-gitea#36402)

And clean up legacy fragile & incorrect logic
# This is the commit message go-gitea#22:

[skip ci] Updated translations via Crowdin

# This is the commit message go-gitea#23:

Fix missing repository id when migrating release attachments (go-gitea#36389)

This PR fixes missed repo_id on the migration of attachments to Gitea.
It also provides a doctor check to fix the dirty data on the database.

Refactor git command context & pipeline (go-gitea#36406)

Less and simpler code, fewer bugs

Allow foreachref parse max tokens from 4*64KB to 4MB (go-gitea#36414)

Fix go-gitea#36408

---------

Signed-off-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

Update material-icon-theme to v5.31.0 (go-gitea#36427)

Update chroma to v2.23.0 (go-gitea#36423)

Fix markdown newline handling during IME composition (go-gitea#36421)

### Summary

Fix incorrect newline handling in markdown editor when using IME input.

### Details

While composing text with an IME, pressing Enter should not trigger
markdown indentation logic.
This change skips indentation handling during composition by checking
`e.isComposing`.

This prevents unexpected line breaks and formatting issues for CJK
users.

[skip ci] Updated translations via Crowdin

Fix typos: unknow -> unknown, pktLineTypeUnknow -> pktLineTypeUnknown (go-gitea#36419)

Fix issue filter menu layout (go-gitea#36426)

Fix go-gitea#36420

Fix spelling (go-gitea#36399)

Signed-off-by: Thomas Beutlich <115483027+thbeu@users.noreply.github.com>

Refactor git command stdio pipe (go-gitea#36422)

Most potential deadlock problems should have been fixed, and new code is
unlikely to cause new problems with the new design.

Also raise the minimum Git version required to 2.6.0 (released in 2015)

Remove `node-check` and `go-check`, support node prerelease versions (go-gitea#36382)

1. Remove those checks for the sake of build performance and because go
and node will fail anyways if their versions are incorrect.
3. Support pre-release Node version for determining NODE_VARS.
2. Update to the chinese READMEs to mention `pnpm` which is already
present in english README.

---------

Co-authored-by: techknowlogick <techknowlogick@gitea.com>
Co-authored-by: Giteabot <teabot@gitea.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

fix: Adjust the unittests

fix: Lint issues

fix: Adjust the swagger config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-update-needed The document needs to be updated synchronously lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files type/refactoring Existing code has been cleaned up. There should be no new functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants