Skip to content

Revert "Do treeless clone for git clone to improve performance" #5908

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

Merged
merged 1 commit into from
Jun 2, 2025

Conversation

Warashi
Copy link
Member

@Warashi Warashi commented Jun 2, 2025

Reverts #5722

The eventwatcher calls repo.CopyToModify every time, regardless of whether the event is registered.
This causes git clone from remote, e.g., GitHub, every time and costs network I/O.

The solution might be to call it CopyToModify only when the modification occurs.
For now, I revert the treeless clone because I don't have enough time to do so.

// execute inspects all Event-definition and handles the events per EventWatcherHandlerType if there are.
func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eventCfgs []eventWatcherConfig) error {
// Copy the repo to another directory to modify local file to avoid reverting previous changes.
tmpDir, err := os.MkdirTemp(w.workingDir, "repo")
if err != nil {
w.logger.Error("failed to create a new temporary directory", zap.Error(err))
return err
}
tmpRepo, err := repo.CopyToModify(filepath.Join(tmpDir, "tmp-repo"))
if err != nil {
w.logger.Error("failed to copy the repository to the temporary directory", zap.Error(err))
return err
}
// nolint: errcheck
defer tmpRepo.Clean()

This reverts commit 7e74937.

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>
@Warashi Warashi force-pushed the revert-5722-git-clone-perf-treeless branch from 3b0e76f to 3fff308 Compare June 2, 2025 06:17
@Warashi Warashi marked this pull request as ready for review June 2, 2025 06:18
@Warashi Warashi requested a review from a team as a code owner June 2, 2025 06:18
Copy link

codecov bot commented Jun 2, 2025

Codecov Report

Attention: Patch coverage is 25.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 27.88%. Comparing base (24a573d) to head (3fff308).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
pkg/git/repo.go 14.28% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5908      +/-   ##
==========================================
- Coverage   27.89%   27.88%   -0.02%     
==========================================
  Files         519      519              
  Lines       55926    55932       +6     
==========================================
- Hits        15603    15596       -7     
- Misses      39125    39136      +11     
- Partials     1198     1200       +2     
Flag Coverage Δ
. 23.73% <25.00%> (-0.02%) ⬇️
.-pkg-app-pipedv1-plugin-example 0.00% <ø> (ø)
.-pkg-app-pipedv1-plugin-kubernetes 63.32% <ø> (ø)
.-pkg-app-pipedv1-plugin-kubernetes_multicluster 64.93% <ø> (ø)
.-pkg-app-pipedv1-plugin-wait 35.51% <ø> (ø)
.-pkg-plugin-sdk 49.33% <ø> (ø)
.-tool-actions-gh-release 19.23% <ø> (ø)
.-tool-actions-plan-preview 22.83% <ø> (ø)
.-tool-codegen-protoc-gen-auth 0.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@ffjlabo ffjlabo left a comment

Choose a reason for hiding this comment

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

Thank you

@Warashi Warashi added the v0.52.1 label Jun 2, 2025
@Warashi Warashi enabled auto-merge (squash) June 2, 2025 06:22
@Warashi Warashi merged commit 18dad94 into master Jun 2, 2025
38 of 40 checks passed
@Warashi Warashi deleted the revert-5722-git-clone-perf-treeless branch June 2, 2025 06:24
@github-actions github-actions bot mentioned this pull request Jun 2, 2025
github-actions bot pushed a commit that referenced this pull request Jun 2, 2025
…" (#5908)

This reverts commit 7e74937.

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>
Signed-off-by: pipecd-bot <[email protected]>
Warashi added a commit that referenced this pull request Jun 2, 2025
* Fix lint errors (#5897)

* Fix import grouping by make lint/go FIX=true

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Fix linter errors by make lint/go FIX=true

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Rewrite long if-else to switch

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Remove unnecessary zero-value initialization

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Remove unnecessary string conversion

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Fix to correctly call deferred functions

log.Fatalf calls os.Exit(1) and deferred functions are not called.
This commit changes the _main function to return an integer and
call os.Exit from main.

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Fix lint errors

log.Fatal calls os.Exit, so the deferred functions are not called.
This commit changes the _main function to return an integer and
call os.Exit from main.
This is a workaround to fix the lint errors.

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Fix lint errors

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Initialize the slice with the correct capacity

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Change return type of doComment to int

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

---------

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>
Signed-off-by: pipecd-bot <[email protected]>

* Add 'title' to distinguish multiple projects in actions-plan-preview (#5905)

* add title to args

Signed-off-by: t-kikuc <[email protected]>

* Add the title to filter latest comments

Signed-off-by: t-kikuc <[email protected]>

* Use hash

Signed-off-by: t-kikuc <[email protected]>

* Fix tests

Signed-off-by: t-kikuc <[email protected]>

* add multibyte test

Signed-off-by: t-kikuc <[email protected]>

---------

Signed-off-by: t-kikuc <[email protected]>
Signed-off-by: pipecd-bot <[email protected]>

* Revert "Do treeless clone for git clone to improve performance (#5722)" (#5908)

This reverts commit 7e74937.

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>
Signed-off-by: pipecd-bot <[email protected]>

* Cut release v0.52.1 (#5909)

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Warashi <[email protected]>
Signed-off-by: pipecd-bot <[email protected]>

---------

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>
Signed-off-by: pipecd-bot <[email protected]>
Signed-off-by: t-kikuc <[email protected]>
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Shinnosuke Sawada-Dazai <[email protected]>
Co-authored-by: Tetsuya KIKUCHI <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Warashi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants