From 47409b2fa044df6f4342181e8f4ee08f9964bc2e Mon Sep 17 00:00:00 2001 From: Giteabot Date: Mon, 26 Feb 2024 17:59:12 +0800 Subject: [PATCH 01/29] Ignore empty repo for CreateRepository in action notifier (#29416) (#29424) Backport #29416 by @yp05327 Fix #29415 Co-authored-by: yp05327 <576951401@qq.com> (cherry picked from commit c758a8afba85fe1847f4d1f5441f6c62b37517ae) --- services/actions/notifier_helper.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 26c86275e9..15fd419e87 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -114,6 +114,9 @@ func notify(ctx context.Context, input *notifyInput) error { log.Debug("ignore executing %v for event %v whose doer is %v", getMethod(ctx), input.Event, input.Doer.Name) return nil } + if input.Repo.IsEmpty { + return nil + } if unit_model.TypeActions.UnitGlobalDisabled() { return nil } From 40318cf9c3a8d6e5e3520326c4bd6ff4ebbfc9c8 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Tue, 27 Feb 2024 16:18:49 +0800 Subject: [PATCH 02/29] Not trigger all jobs any more, when re-running the first job (#29439) (#29441) Backport #29439 by @sillyguodong Previously, it will be treated as "re-run all jobs" when `jobIndex == 0`. So when you click re-run button on the first job, it triggers all the jobs actually. Caused by #26535. Co-authored-by: sillyguodong <33891828+sillyguodong@users.noreply.github.com> (cherry picked from commit 9456deb512db59025cae26d82812ff880c5ea3bc) --- routers/web/repo/actions/view.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index a9c2858303..02f8cfbf1b 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -12,6 +12,7 @@ import ( "io" "net/http" "net/url" + "strconv" "strings" "time" @@ -260,10 +261,14 @@ func ViewPost(ctx *context_module.Context) { } // Rerun will rerun jobs in the given run -// jobIndex = 0 means rerun all jobs +// If jobIndexStr is a blank string, it means rerun all jobs func Rerun(ctx *context_module.Context) { runIndex := ctx.ParamsInt64("run") - jobIndex := ctx.ParamsInt64("job") + jobIndexStr := ctx.Params("job") + var jobIndex int64 + if jobIndexStr != "" { + jobIndex, _ = strconv.ParseInt(jobIndexStr, 10, 64) + } run, err := actions_model.GetRunByIndex(ctx, ctx.Repo.Repository.ID, runIndex) if err != nil { @@ -284,7 +289,7 @@ func Rerun(ctx *context_module.Context) { return } - if jobIndex != 0 { + if jobIndexStr != "" { jobs = []*actions_model.ActionRunJob{job} } From 40c602df9bedcd3f5168fce4b42019f85183e837 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 27 Feb 2024 19:11:38 +0800 Subject: [PATCH 03/29] Fix template bug (#27581) (#29446) Fix #29152 Backport #27581 (cherry picked from commit dc48eb070b2e4143739b7ab22bb9e172af90106d) --- templates/user/dashboard/feeds.tmpl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/templates/user/dashboard/feeds.tmpl b/templates/user/dashboard/feeds.tmpl index a1ac3d8848..da796fc275 100644 --- a/templates/user/dashboard/feeds.tmpl +++ b/templates/user/dashboard/feeds.tmpl @@ -83,6 +83,7 @@ {{if .GetOpType.InActions "commit_repo" "mirror_sync_push"}} {{$push := ActionContent2Commits .}} {{$repoLink := (.GetRepoLink ctx)}} + {{$repo := .Repo}}
{{range $push.Commits}} {{$commitLink := printf "%s/commit/%s" $repoLink .Sha1}} @@ -90,7 +91,7 @@ {{ShortSha .Sha1}} - {{RenderCommitMessage $.Context .Message $.ComposeMetas}} + {{RenderCommitMessage $.Context .Message $repo.ComposeMetas}}
{{end}} From d97cd15a1b4b53b285521f90ca6d131c421f28ed Mon Sep 17 00:00:00 2001 From: Giteabot Date: Tue, 27 Feb 2024 23:33:03 +0800 Subject: [PATCH 04/29] Fix missed return (#29450) (#29453) Backport #29450 by @lunny Co-authored-by: Lunny Xiao (cherry picked from commit 2df38af752c13ca02a899d6a53848c68259d3336) --- routers/api/v1/repo/file.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 9ee10f6ce1..810dbe97f1 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -654,6 +654,7 @@ func UpdateFile(ctx *context.APIContext) { apiOpts := web.GetForm(ctx).(*api.UpdateFileOptions) if ctx.Repo.Repository.IsEmpty { ctx.Error(http.StatusUnprocessableEntity, "RepoIsEmpty", fmt.Errorf("repo is empty")) + return } if apiOpts.BranchName == "" { From 813577aee138d60814d2ab9cd1a1f836afc8771a Mon Sep 17 00:00:00 2001 From: Giteabot Date: Wed, 28 Feb 2024 19:22:31 +0800 Subject: [PATCH 05/29] The job should always run when `if` is `always()` (#29464) (#29469) Backport #29464 by @Zettat123 Fix #27906 According to GitHub's [documentation](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idneeds), a job should always run when its `if` is `always()` > If you would like a job to run even if a job it is dependent on did not succeed, use the `always()` conditional expression in `jobs..if`. Co-authored-by: Zettat123 (cherry picked from commit eabcfd3f7d9321fcf03e52977c178a96627a68da) --- services/actions/job_emitter.go | 21 ++++++++++- services/actions/job_emitter_test.go | 56 ++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/services/actions/job_emitter.go b/services/actions/job_emitter.go index f7ec615364..e234469348 100644 --- a/services/actions/job_emitter.go +++ b/services/actions/job_emitter.go @@ -7,12 +7,14 @@ import ( "context" "errors" "fmt" + "strings" actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/queue" + "github.com/nektos/act/pkg/jobparser" "xorm.io/builder" ) @@ -76,12 +78,15 @@ func checkJobsOfRun(ctx context.Context, runID int64) error { type jobStatusResolver struct { statuses map[int64]actions_model.Status needs map[int64][]int64 + jobMap map[int64]*actions_model.ActionRunJob } func newJobStatusResolver(jobs actions_model.ActionJobList) *jobStatusResolver { idToJobs := make(map[string][]*actions_model.ActionRunJob, len(jobs)) + jobMap := make(map[int64]*actions_model.ActionRunJob) for _, job := range jobs { idToJobs[job.JobID] = append(idToJobs[job.JobID], job) + jobMap[job.ID] = job } statuses := make(map[int64]actions_model.Status, len(jobs)) @@ -97,6 +102,7 @@ func newJobStatusResolver(jobs actions_model.ActionJobList) *jobStatusResolver { return &jobStatusResolver{ statuses: statuses, needs: needs, + jobMap: jobMap, } } @@ -135,7 +141,20 @@ func (r *jobStatusResolver) resolve() map[int64]actions_model.Status { if allSucceed { ret[id] = actions_model.StatusWaiting } else { - ret[id] = actions_model.StatusSkipped + // If a job's "if" condition is "always()", the job should always run even if some of its dependencies did not succeed. + // See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idneeds + always := false + if wfJobs, _ := jobparser.Parse(r.jobMap[id].WorkflowPayload); len(wfJobs) == 1 { + _, wfJob := wfJobs[0].Job() + expr := strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(wfJob.If.Value, "${{"), "}}")) + always = expr == "always()" + } + + if always { + ret[id] = actions_model.StatusWaiting + } else { + ret[id] = actions_model.StatusSkipped + } } } } diff --git a/services/actions/job_emitter_test.go b/services/actions/job_emitter_test.go index e81aa61d80..038df7d4f8 100644 --- a/services/actions/job_emitter_test.go +++ b/services/actions/job_emitter_test.go @@ -70,6 +70,62 @@ func Test_jobStatusResolver_Resolve(t *testing.T) { }, want: map[int64]actions_model.Status{}, }, + { + name: "with ${{ always() }} condition", + jobs: actions_model.ActionJobList{ + {ID: 1, JobID: "job1", Status: actions_model.StatusFailure, Needs: []string{}}, + {ID: 2, JobID: "job2", Status: actions_model.StatusBlocked, Needs: []string{"job1"}, WorkflowPayload: []byte( + ` +name: test +on: push +jobs: + job2: + runs-on: ubuntu-latest + needs: job1 + if: ${{ always() }} + steps: + - run: echo "always run" +`)}, + }, + want: map[int64]actions_model.Status{2: actions_model.StatusWaiting}, + }, + { + name: "with always() condition", + jobs: actions_model.ActionJobList{ + {ID: 1, JobID: "job1", Status: actions_model.StatusFailure, Needs: []string{}}, + {ID: 2, JobID: "job2", Status: actions_model.StatusBlocked, Needs: []string{"job1"}, WorkflowPayload: []byte( + ` +name: test +on: push +jobs: + job2: + runs-on: ubuntu-latest + needs: job1 + if: always() + steps: + - run: echo "always run" +`)}, + }, + want: map[int64]actions_model.Status{2: actions_model.StatusWaiting}, + }, + { + name: "without always() condition", + jobs: actions_model.ActionJobList{ + {ID: 1, JobID: "job1", Status: actions_model.StatusFailure, Needs: []string{}}, + {ID: 2, JobID: "job2", Status: actions_model.StatusBlocked, Needs: []string{"job1"}, WorkflowPayload: []byte( + ` +name: test +on: push +jobs: + job2: + runs-on: ubuntu-latest + needs: job1 + steps: + - run: echo "not always run" +`)}, + }, + want: map[int64]actions_model.Status{2: actions_model.StatusSkipped}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 0b287c4d4cb16875d07ed4702006115ea429cead Mon Sep 17 00:00:00 2001 From: Giteabot Date: Wed, 28 Feb 2024 23:25:53 +0800 Subject: [PATCH 06/29] Fix URL calculation in clone input box (#29470) (#29473) Backport #29470 by @silverwind Ported the function as-is and added comments so we don't forget about this in the future. Fixes: https://github.com/go-gitea/gitea/issues/29462 Co-authored-by: silverwind (cherry picked from commit 222f93822eb4d03f5001ef665377a115bc27ccb6) --- templates/repo/clone_script.tmpl | 22 +++++++++++++++------- web_src/js/webcomponents/GiteaOriginUrl.js | 5 +++-- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/templates/repo/clone_script.tmpl b/templates/repo/clone_script.tmpl index 0797b400d8..8b951d8fc4 100644 --- a/templates/repo/clone_script.tmpl +++ b/templates/repo/clone_script.tmpl @@ -24,14 +24,22 @@ const btn = isSSH ? sshBtn : httpsBtn; if (!btn) return; - let link = btn.getAttribute('data-link'); - if (link.startsWith('http://') || link.startsWith('https://')) { - // use current protocol/host as the clone link - const url = new URL(link); - url.protocol = window.location.protocol; - url.host = window.location.host; - link = url.toString(); + // NOTE: Keep this function in sync with the one in the js folder + function toOriginUrl(urlStr) { + try { + if (urlStr.startsWith('http://') || urlStr.startsWith('https://') || urlStr.startsWith('/')) { + const {origin, protocol, hostname, port} = window.location; + const url = new URL(urlStr, origin); + url.protocol = protocol; + url.hostname = hostname; + url.port = port || (protocol === 'https:' ? '443' : '80'); + return url.toString(); + } + } catch {} + return urlStr; } + const link = toOriginUrl(btn.getAttribute('data-link')); + for (const el of document.getElementsByClassName('js-clone-url')) { el[el.nodeName === 'INPUT' ? 'value' : 'textContent'] = link; } diff --git a/web_src/js/webcomponents/GiteaOriginUrl.js b/web_src/js/webcomponents/GiteaOriginUrl.js index 5d71d95c60..6e6f84d739 100644 --- a/web_src/js/webcomponents/GiteaOriginUrl.js +++ b/web_src/js/webcomponents/GiteaOriginUrl.js @@ -1,7 +1,8 @@ -// Convert an absolute or relative URL to an absolute URL with the current origin +// Convert an absolute or relative URL to an absolute URL with the current origin. It only +// processes absolute HTTP/HTTPS URLs or relative URLs like '/xxx' or '//host/xxx'. +// NOTE: Keep this function in sync with clone_script.tmpl export function toOriginUrl(urlStr) { try { - // only process absolute HTTP/HTTPS URL or relative URLs ('/xxx' or '//host/xxx') if (urlStr.startsWith('http://') || urlStr.startsWith('https://') || urlStr.startsWith('/')) { const {origin, protocol, hostname, port} = window.location; const url = new URL(urlStr, origin); From 8babbe7ccf4e6b73aa8b67dc88c470c516f0b6cc Mon Sep 17 00:00:00 2001 From: Giteabot Date: Thu, 29 Feb 2024 06:13:49 +0800 Subject: [PATCH 07/29] Fix counter display number incorrectly displayed on the page (#29448) (#29478) Backport #29448 by @charles7668 issue : #28239 The counter number script uses the 'checkbox' attribute to determine whether an item is selected or not. However, the input event only increments the counter value, and when more items are displayed, it does not update all previously loaded items. As a result, the display becomes incorrect because it triggers the update counter script, but checkboxes that are selected without the 'checked' attribute are not counted Co-authored-by: charles <30816317+charles7668@users.noreply.github.com> (cherry picked from commit 5477728282de19b1638691b88449b1933ed5a4d8) --- web_src/js/features/pull-view-file.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/web_src/js/features/pull-view-file.js b/web_src/js/features/pull-view-file.js index 86b65f68cf..f68c8887bb 100644 --- a/web_src/js/features/pull-view-file.js +++ b/web_src/js/features/pull-view-file.js @@ -44,9 +44,11 @@ export function initViewedCheckboxListenerFor() { // Mark the file as viewed visually - will especially change the background if (this.checked) { form.classList.add(viewedStyleClass); + checkbox.setAttribute('checked', ''); prReview.numberOfViewedFiles++; } else { form.classList.remove(viewedStyleClass); + checkbox.removeAttribute('checked'); prReview.numberOfViewedFiles--; } From 581261efb1170dbbaadaaa4e98f785e3a7e8b16b Mon Sep 17 00:00:00 2001 From: Giteabot Date: Thu, 29 Feb 2024 08:22:53 +0800 Subject: [PATCH 08/29] Fix/Improve `processWindowErrorEvent` (#29407) (#29480) Backport #29407 by @silverwind - `e.error` can be undefined in some cases which would raise an error inside this error handler, fixed that. - The displayed message mentions looking into the console, but in my case of error from `ResizeObserver` there was nothing there, so add this logging. I think this logging was once there but got lost during refactoring. Co-authored-by: silverwind (cherry picked from commit 9abba8c11a2eddfa8da25946fdd43be8d0c53689) --- web_src/js/bootstrap.js | 57 ++++++++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/web_src/js/bootstrap.js b/web_src/js/bootstrap.js index e46c91e5e6..c0047b0ac2 100644 --- a/web_src/js/bootstrap.js +++ b/web_src/js/bootstrap.js @@ -1,5 +1,6 @@ // DO NOT IMPORT window.config HERE! -// to make sure the error handler always works, we should never import `window.config`, because some user's custom template breaks it. +// to make sure the error handler always works, we should never import `window.config`, because +// some user's custom template breaks it. // This sets up the URL prefix used in webpack's chunk loading. // This file must be imported before any lazy-loading is being attempted. @@ -26,29 +27,42 @@ export function showGlobalErrorMessage(msg) { } /** - * @param {ErrorEvent} e + * @param {ErrorEvent|PromiseRejectionEvent} event - Event + * @param {string} event.message - Only present on ErrorEvent + * @param {string} event.error - Only present on ErrorEvent + * @param {string} event.type - Only present on ErrorEvent + * @param {string} event.filename - Only present on ErrorEvent + * @param {number} event.lineno - Only present on ErrorEvent + * @param {number} event.colno - Only present on ErrorEvent + * @param {string} event.reason - Only present on PromiseRejectionEvent + * @param {number} event.promise - Only present on PromiseRejectionEvent */ -function processWindowErrorEvent(e) { - const err = e.error ?? e.reason; +function processWindowErrorEvent({error, reason, message, type, filename, lineno, colno}) { + const err = error ?? reason; const assetBaseUrl = String(new URL(__webpack_public_path__, window.location.origin)); + const {runModeIsProd} = window.config ?? {}; - // error is likely from browser extension or inline script. Do not show these in production builds. - if (!err.stack?.includes(assetBaseUrl) && window.config?.runModeIsProd) return; - - let message; - if (e.type === 'unhandledrejection') { - message = `JavaScript promise rejection: ${err.message}.`; - } else { - message = `JavaScript error: ${e.message} (${e.filename} @ ${e.lineno}:${e.colno}).`; + // `error` and `reason` are not guaranteed to be errors. If the value is falsy, it is likly a + // non-critical event from the browser. We log them but don't show them to users. Examples: + // - https://developer.mozilla.org/en-US/docs/Web/API/ResizeObserver#observation_errors + // - https://github.com/mozilla-mobile/firefox-ios/issues/10817 + // - https://github.com/go-gitea/gitea/issues/20240 + if (!err) { + if (message) console.error(new Error(message)); + if (runModeIsProd) return; } - if (!e.error && e.lineno === 0 && e.colno === 0 && e.filename === '' && window.navigator.userAgent.includes('FxiOS/')) { - // At the moment, Firefox (iOS) (10x) has an engine bug. See https://github.com/go-gitea/gitea/issues/20240 - // If a script inserts a newly created (and content changed) element into DOM, there will be a nonsense error event reporting: Script error: line 0, col 0. - return; // ignore such nonsense error event + // If the error stack trace does not include the base URL of our script assets, it likely came + // from a browser extension or inline script. Do not show such errors in production. + if (err instanceof Error && !err.stack?.includes(assetBaseUrl) && runModeIsProd) { + return; } - showGlobalErrorMessage(`${message} Open browser console to see more details.`); + let msg = err?.message ?? message; + if (lineno) msg += ` (${filename} @ ${lineno}:${colno})`; + const dot = msg.endsWith('.') ? '' : '.'; + const renderedType = type === 'unhandledrejection' ? 'promise rejection' : type; + showGlobalErrorMessage(`JavaScript ${renderedType}: ${msg}${dot} Open browser console to see more details.`); } function initGlobalErrorHandler() { @@ -59,13 +73,14 @@ function initGlobalErrorHandler() { if (!window.config) { showGlobalErrorMessage(`Gitea JavaScript code couldn't run correctly, please check your custom templates`); } - // we added an event handler for window error at the very beginning of