Skip to content

Comments

Refactor git command stderr handling#36402

Merged
lunny merged 1 commit intogo-gitea:mainfrom
wxiaoguang:fix-git-cmd
Jan 18, 2026
Merged

Refactor git command stderr handling#36402
lunny merged 1 commit intogo-gitea:mainfrom
wxiaoguang:fix-git-cmd

Conversation

@wxiaoguang
Copy link
Contributor

@wxiaoguang wxiaoguang commented Jan 18, 2026

And clean up legacy fragile & incorrect logic

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 18, 2026
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin labels Jan 18, 2026
@wxiaoguang wxiaoguang mentioned this pull request Jan 18, 2026
Merged
@wxiaoguang wxiaoguang force-pushed the fix-git-cmd branch 4 times, most recently from c6ff875 to 353ba6a Compare January 18, 2026 11:19
@wxiaoguang wxiaoguang added this to the 1.26.0 milestone Jan 18, 2026
@wxiaoguang wxiaoguang force-pushed the fix-git-cmd branch 3 times, most recently from 4ad1a64 to 2ab919d Compare January 18, 2026 11:45
@wxiaoguang wxiaoguang requested a review from Copilot January 18, 2026 12:06
Copilot AI reviewed Jan 18, 2026
View reviewed changes
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 pull request refactors git command stderr handling across the entire codebase, replacing fragile manual stderr buffer management with a new unified approach using RunWithStderr and RunStdString methods. The refactoring removes the deprecated ConcatenateError function and eliminates numerous instances of manual buffer creation and management for stderr output.

Changes:

  • Introduced new RunWithStderr(), StartWithStderr(), and WaitWithStderr() methods in the git command infrastructure that automatically manage stderr collection
  • Updated RunStdString() and RunStdBytes() to use the new managed stderr approach internally
  • Removed manual stderr buffer creation and WithStderr() calls across 50+ files
  • Simplified error messages by relying on the error's embedded stderr content accessed via err.Stderr()
  • Removed helper functions GitRemotePrune and GitRemoteUpdatePrune in favor of direct command usage

Reviewed changes

Copilot reviewed 69 out of 69 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
modules/git/gitcmd/command.go Core implementation of managed stderr handling with new RunWithStderr methods
modules/git/gitcmd/utils.go Removed deprecated ConcatenateError function
modules/git/gitcmd/command_test.go Updated test expectations for new error message format
modules/gitrepo/command.go Added RunCmdWithStderr wrapper and updated RunCmdString/RunCmdBytes signatures
modules/gitrepo/remote.go Removed GitRemotePrune and GitRemoteUpdatePrune helper functions
modules/gitrepo/*.go (multiple) Updated all command wrappers to return new signature types
modules/git/repo*.go (multiple) Replaced manual stderr buffers with RunWithStderr/RunStdString
modules/git/pipeline/*.go Simplified pipeline error handling with managed stderr
modules/git/attribute/*.go Updated attribute checking to use managed stderr
services/repository/*.go Removed manual stderr buffer creation and simplified error messages
services/pull/*.go Updated merge/rebase operations to use err.Stderr() instead of manual buffers
services/mirror/mirror_pull.go Refactored mirror sync to use managed stderr and simplified prune operations
services/migrations/gitea_uploader.go Updated git fetch operations to new signature
routers/web/repo/*.go Updated git operations in HTTP handlers
routers/private/hook_pre_receive.go Updated pre-receive hook git commands
cmd/hook.go Updated post-receive hook command
tests/integration/pull_merge_test.go Updated test to use new RunStdString signature
.golangci.yml Added linter rule to prevent use of deprecated errors package

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

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 18, 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 18, 2026
@lunny lunny merged commit 72be55f into go-gitea:main Jan 18, 2026
24 checks passed
@wxiaoguang wxiaoguang deleted the fix-git-cmd branch January 19, 2026 00:32
@wxiaoguang wxiaoguang mentioned this pull request Jan 19, 2026
Closed
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jan 21, 2026
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
77b0552 
* 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)
  ...
ZPascal added a commit to ZPascal/gitea that referenced this pull request Jan 22, 2026
@ZPascal
# This is a combination of 23 commits.
23ed160 
# 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

Reviewers

Copilot code review Copilot Copilot left review comments

@lunny lunny lunny approved these changes

+1 more reviewer

@TheFox0x7 TheFox0x7 TheFox0x7 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin modifies/go Pull requests that update Go code modifies/internal topic/code-linting

Projects

None yet

Milestone

1.26.0

Development

Successfully merging this pull request may close these issues.

4 participants

@wxiaoguang @lunny @TheFox0x7 @GiteaBot