Skip to content

Link to executed workflow #31906

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

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
1 change: 1 addition & 0 deletions models/actions/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ func (run *ActionRun) Link() string {
return fmt.Sprintf("%s/actions/runs/%d", run.Repo.Link(), run.Index)
}

// WorkflowLink return the url to the actions list filtered for this runs workflow
func (run *ActionRun) WorkflowLink() string {
if run.Repo == nil {
return ""
Expand Down
38 changes: 38 additions & 0 deletions modules/actions/workflows.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ package actions

import (
"bytes"
"fmt"
"io"
"strings"

actions_model "code.gitea.io/gitea/models/actions"
Copy link
Member

Choose a reason for hiding this comment

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

modules is the wrong place for this.
Go does not allow import cycles.
Please do not add any imports to models from modules.

Copy link
Contributor Author

@william-allspice william-allspice Sep 4, 2024

Choose a reason for hiding this comment

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

modules is the wrong place for this.

@delvh can you expand on what you are asking for?
It sounds like you are asking to move this function and the ListWorkflows function to models? In my humble opinion, I don't think this makes sense because there is no existing concept of a workflow in the database which to me is the equivalent of the models. Happy to hear a different opinion.

I could refactor a lot of the workflow logic, including ListWorkflows, to live in a new file called models/actions/workflows. But it feels odd since there is no database table for a workflow and would violate this principle:

models: Contains the data structures used by xorm to construct database tables. It also contains functions to query and update the database. Dependencies to other Gitea code should be avoided. You can make exceptions in cases such as logging.

Outlined here: https://docs.gitea.com/contributing/guidelines-backend

This module is already dependent on the actions model here: https://github.com/go-gitea/gitea/blob/main/modules/actions/task_state.go#L7

Copy link
Member

Choose a reason for hiding this comment

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

This module is already dependent on the actions model here: https://github.com/go-gitea/gitea/blob/main/modules/actions/task_state.go#L7

🤮 Okay, that is one of the places we didn't fix yet.
We are still working on removing all the historically grown errors.
For the time being, I think it is better not to introduce any already known technical debt.

As to where to add it, I would have added it as a type method on actions_model.ActionRun which is defined in models/actions/run.go

"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
api "code.gitea.io/gitea/modules/structs"
Expand Down Expand Up @@ -69,6 +71,42 @@ func ListWorkflows(commit *git.Commit) (git.Entries, error) {
return ret, nil
}

// GetRunsWorkflowFileLink return url for the source of the workflow file for an ActionRun
func GetRunsWorkflowFileLink(run *actions_model.ActionRun, gitRepo *git.Repository) string {
if run.Repo == nil || run.CommitSHA == "" {
return ""
}

commit, err := gitRepo.GetCommit(run.CommitSHA)
if err != nil {
return ""
}

entries, err := ListWorkflows(commit)
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I think I could understand the problem now.

The proper approach should be like this: add a "WorkflowFilePath" column into the "Actions" model and just use it.

It shouldn't call ListWorkflows for viewing action workflows, otherwise there will be a performance problem.

if err != nil {
return ""
}

var workflowEntry *git.TreeEntry
for _, entry := range entries {
if entry.Name() == run.WorkflowID {
workflowEntry = entry
break
}
}

if workflowEntry == nil {
return ""
}

workflowFilePath := workflowEntry.GetPathInRepo()
if workflowFilePath == "" {
return ""
}

return fmt.Sprintf("%s/src/commit/%s/%s", run.Repo.Link(), run.CommitSHA, workflowFilePath)
Copy link
Contributor

Choose a reason for hiding this comment

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

It needs proper escaping, otherwise the link might be invalid

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, we can't influence the workflow file

}

func GetContentFromEntry(entry *git.TreeEntry) ([]byte, error) {
f, err := entry.Blob().DataAsync()
if err != nil {
Expand Down
29 changes: 29 additions & 0 deletions modules/git/tree_entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"io"
"sort"
"strings"

"code.gitea.io/gitea/modules/log"
)

// Type returns the type of the entry (commit, tree, blob)
Expand Down Expand Up @@ -179,3 +181,30 @@ func (tes Entries) Sort() {
func (tes Entries) CustomSort(cmp func(s1, s2 string) bool) {
sort.Sort(customSortableEntries{cmp, tes})
}

Copy link
Member

Choose a reason for hiding this comment

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

Adding a note for other reviewers: this file doesn't differentiate between go-git and non-go-git impl, so it won't need multiple implementations based on which backend is used.

Copy link
Contributor

Choose a reason for hiding this comment

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

Total side-tangent, but does anyone use go-git?

Copy link
Member

Choose a reason for hiding this comment

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

go-git version has been used in almost all Windows users.

// GetPathInRepo returns the relative path in the tree to this entry
func (te *TreeEntry) GetPathInRepo() string {
Copy link
Contributor

@wxiaoguang wxiaoguang Sep 5, 2024

Choose a reason for hiding this comment

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

This file was organized by this:

TreeEntry code
Entries code

But now it becomes:

TreeEntry code
Entries code
TreeEntry code

The newly added code should be in the existing "TreeEntry code" section.

if te == nil {
return ""
}

path := te.Name()
current := te.ptree

for current != nil && current.ptree != nil {
entries, err := current.ptree.ListEntries()
if err != nil {
log.Error("Failed to climb git tree %v", err)
return ""
}
for _, entry := range entries {
if entry.ID.String() == current.ID.String() {
path = entry.Name() + "/" + path
break
}
}
current = current.ptree
}
Comment on lines +194 to +207
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a really inefficient way to gather the full path of a file.
However, I know that this function is drastically needed, also for other functionality (see for example #28835 which got stalled exactly as I wasn't sure how to implement this)._
Thus, I'll allow it for now as long as we get it merged.
We can still improve its performance in the future with a precalculated path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I think a TreeEntry should have its absolute path. But, felt out of scope for this requested feature.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also agree with delvh, we can't do so. #31906 (comment)


return path
}
55 changes: 55 additions & 0 deletions modules/git/tree_entry_gogit_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

//go:build gogit

package git

import (
"testing"

"github.com/go-git/go-git/v5/plumbing/filemode"
"github.com/go-git/go-git/v5/plumbing/object"
"github.com/stretchr/testify/assert"
)

func getTestEntries() Entries {
return Entries{
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "v1.0", Mode: filemode.Dir}},
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "v2.0", Mode: filemode.Dir}},
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "v2.1", Mode: filemode.Dir}},
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "v2.12", Mode: filemode.Dir}},
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "v2.2", Mode: filemode.Dir}},
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "v12.0", Mode: filemode.Dir}},
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "abc", Mode: filemode.Regular}},
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "bcd", Mode: filemode.Regular}},
}
}

func TestEntriesSort(t *testing.T) {
entries := getTestEntries()
entries.Sort()
assert.Equal(t, "v1.0", entries[0].Name())
assert.Equal(t, "v12.0", entries[1].Name())
assert.Equal(t, "v2.0", entries[2].Name())
assert.Equal(t, "v2.1", entries[3].Name())
assert.Equal(t, "v2.12", entries[4].Name())
assert.Equal(t, "v2.2", entries[5].Name())
assert.Equal(t, "abc", entries[6].Name())
assert.Equal(t, "bcd", entries[7].Name())
}

func TestEntriesCustomSort(t *testing.T) {
entries := getTestEntries()
entries.CustomSort(func(s1, s2 string) bool {
return s1 > s2
})
assert.Equal(t, "v2.2", entries[0].Name())
assert.Equal(t, "v2.12", entries[1].Name())
assert.Equal(t, "v2.1", entries[2].Name())
assert.Equal(t, "v2.0", entries[3].Name())
assert.Equal(t, "v12.0", entries[4].Name())
assert.Equal(t, "v1.0", entries[5].Name())
assert.Equal(t, "bcd", entries[6].Name())
assert.Equal(t, "abc", entries[7].Name())
}
72 changes: 27 additions & 45 deletions modules/git/tree_entry_test.go
Original file line number Diff line number Diff line change
@@ -1,59 +1,14 @@
// Copyright 2017 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

//go:build gogit

package git

import (
"testing"

"github.com/go-git/go-git/v5/plumbing/filemode"
"github.com/go-git/go-git/v5/plumbing/object"
"github.com/stretchr/testify/assert"
)

func getTestEntries() Entries {
return Entries{
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "v1.0", Mode: filemode.Dir}},
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "v2.0", Mode: filemode.Dir}},
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "v2.1", Mode: filemode.Dir}},
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "v2.12", Mode: filemode.Dir}},
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "v2.2", Mode: filemode.Dir}},
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "v12.0", Mode: filemode.Dir}},
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "abc", Mode: filemode.Regular}},
&TreeEntry{gogitTreeEntry: &object.TreeEntry{Name: "bcd", Mode: filemode.Regular}},
}
}

func TestEntriesSort(t *testing.T) {
entries := getTestEntries()
entries.Sort()
assert.Equal(t, "v1.0", entries[0].Name())
assert.Equal(t, "v12.0", entries[1].Name())
assert.Equal(t, "v2.0", entries[2].Name())
assert.Equal(t, "v2.1", entries[3].Name())
assert.Equal(t, "v2.12", entries[4].Name())
assert.Equal(t, "v2.2", entries[5].Name())
assert.Equal(t, "abc", entries[6].Name())
assert.Equal(t, "bcd", entries[7].Name())
}

func TestEntriesCustomSort(t *testing.T) {
entries := getTestEntries()
entries.CustomSort(func(s1, s2 string) bool {
return s1 > s2
})
assert.Equal(t, "v2.2", entries[0].Name())
assert.Equal(t, "v2.12", entries[1].Name())
assert.Equal(t, "v2.1", entries[2].Name())
assert.Equal(t, "v2.0", entries[3].Name())
assert.Equal(t, "v12.0", entries[4].Name())
assert.Equal(t, "v1.0", entries[5].Name())
assert.Equal(t, "bcd", entries[6].Name())
assert.Equal(t, "abc", entries[7].Name())
}

func TestFollowLink(t *testing.T) {
r, err := openRepositoryWithDefaultContext("tests/repos/repo1_bare")
assert.NoError(t, err)
Expand Down Expand Up @@ -100,3 +55,30 @@ func TestFollowLink(t *testing.T) {
_, err = target.FollowLink()
assert.EqualError(t, err, "link_short: broken link")
}

func TestGetPathInRepo(t *testing.T) {
r, err := openRepositoryWithDefaultContext("tests/repos/repo1_bare")
assert.NoError(t, err)
defer r.Close()

commit, err := r.GetCommit("37991dec2c8e592043f47155ce4808d4580f9123")
assert.NoError(t, err)

// nested entry
entry, err := commit.Tree.GetTreeEntryByPath("foo/bar/link_to_hello")
assert.NoError(t, err)
path := entry.GetPathInRepo()
assert.Equal(t, "foo/bar/link_to_hello", path)

// folder
entry, err = commit.Tree.GetTreeEntryByPath("foo/bar")
assert.NoError(t, err)
path = entry.GetPathInRepo()
assert.Equal(t, "foo/bar", path)

// top level file
entry, err = commit.Tree.GetTreeEntryByPath("file2.txt")
assert.NoError(t, err)
path = entry.GetPathInRepo()
assert.Equal(t, "file2.txt", path)
}
2 changes: 2 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -3697,6 +3697,7 @@ runs.no_workflows.documentation = For more information on Gitea Actions, see <a
runs.no_runs = The workflow has no runs yet.
runs.empty_commit_message = (empty commit message)
runs.expire_log_message = Logs have been purged because they were too old.
runs.details = Run details

workflow.disable = Disable Workflow
workflow.disable_success = Workflow '%s' disabled successfully.
Expand All @@ -3708,6 +3709,7 @@ workflow.not_found = Workflow '%s' not found.
workflow.run_success = Workflow '%s' run successfully.
workflow.from_ref = Use workflow from
workflow.has_workflow_dispatch = This workflow has a workflow_dispatch event trigger.
workflow.file = Workflow file

need_approval_desc = Need approval to run workflows for fork pull request.

Expand Down
2 changes: 2 additions & 0 deletions routers/web/repo/actions/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ type ViewResponse struct {
CanRerun bool `json:"canRerun"`
CanDeleteArtifact bool `json:"canDeleteArtifact"`
Done bool `json:"done"`
WorkflowFileLink string `json:"workflowFileLink"`
WorkflowID string `json:"workflowID"`
WorkflowLink string `json:"workflowLink"`
IsSchedule bool `json:"isSchedule"`
Expand Down Expand Up @@ -170,6 +171,7 @@ func ViewPost(ctx *context_module.Context) {
resp.State.Run.CanRerun = run.Status.IsDone() && ctx.Repo.CanWrite(unit.TypeActions)
resp.State.Run.CanDeleteArtifact = run.Status.IsDone() && ctx.Repo.CanWrite(unit.TypeActions)
resp.State.Run.Done = run.Status.IsDone()
resp.State.Run.WorkflowFileLink = actions.GetRunsWorkflowFileLink(run, ctx.Repo.GitRepo)
resp.State.Run.WorkflowID = run.WorkflowID
resp.State.Run.WorkflowLink = run.WorkflowLink()
resp.State.Run.IsSchedule = run.IsSchedule()
Expand Down
2 changes: 2 additions & 0 deletions templates/repo/actions/view.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
data-locale-rerun-all="{{ctx.Locale.Tr "rerun_all"}}"
data-locale-runs-scheduled="{{ctx.Locale.Tr "actions.runs.scheduled"}}"
data-locale-runs-commit="{{ctx.Locale.Tr "actions.runs.commit"}}"
data-locale-runs-details="{{ctx.Locale.Tr "actions.runs.details"}}"
data-locale-runs-pushed-by="{{ctx.Locale.Tr "actions.runs.pushed_by"}}"
data-locale-status-unknown="{{ctx.Locale.Tr "actions.status.unknown"}}"
data-locale-status-waiting="{{ctx.Locale.Tr "actions.status.waiting"}}"
Expand All @@ -27,6 +28,7 @@
data-locale-show-log-seconds="{{ctx.Locale.Tr "show_log_seconds"}}"
data-locale-show-full-screen="{{ctx.Locale.Tr "show_full_screen"}}"
data-locale-download-logs="{{ctx.Locale.Tr "download_logs"}}"
data-locale-workflow-file="{{ctx.Locale.Tr "actions.workflow.file"}}"
>
</div>
</div>
Expand Down
39 changes: 27 additions & 12 deletions web_src/js/components/RepoActionView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const sfc = {
done: false,
workflowID: '',
workflowLink: '',
workflowFileLink: '',
isSchedule: false,
jobs: [
// {
Expand Down Expand Up @@ -350,10 +351,12 @@ export function initRepositoryActionView() {
artifactsTitle: el.getAttribute('data-locale-artifacts-title'),
areYouSure: el.getAttribute('data-locale-are-you-sure'),
confirmDeleteArtifact: el.getAttribute('data-locale-confirm-delete-artifact'),
runDetails: el.getAttribute('data-locale-runs-details'),
showTimeStamps: el.getAttribute('data-locale-show-timestamps'),
showLogSeconds: el.getAttribute('data-locale-show-log-seconds'),
showFullScreen: el.getAttribute('data-locale-show-full-screen'),
downloadLogs: el.getAttribute('data-locale-download-logs'),
workflowFile: el.getAttribute('data-locale-workflow-file'),
status: {
unknown: el.getAttribute('data-locale-status-unknown'),
waiting: el.getAttribute('data-locale-status-waiting'),
Expand Down Expand Up @@ -421,17 +424,29 @@ export function initRepositoryActionView() {
</a>
</div>
</div>
<div class="job-artifacts" v-if="artifacts.length > 0">
<div class="job-artifacts-title">
<div class="left-side-section" v-if="artifacts.length > 0">
<div class="left-side-section-title">
{{ locale.artifactsTitle }}
</div>
<ul class="job-artifacts-list">
<li class="job-artifacts-item" v-for="artifact in artifacts" :key="artifact.name">
<a class="job-artifacts-link" target="_blank" :href="run.link+'/artifacts/'+artifact.name">
<SvgIcon name="octicon-file" class="ui text black job-artifacts-icon"/>{{ artifact.name }}
<ul class="left-side-section-list">
<li class="left-side-section-item" v-for="artifact in artifacts" :key="artifact.name">
<a class="left-side-section-link" target="_blank" :href="run.link+'/artifacts/'+artifact.name">
<SvgIcon name="octicon-file" class="ui text black left-side-section-icon"/>{{ artifact.name }}
</a>
<a v-if="run.canDeleteArtifact" @click="deleteArtifact(artifact.name)" class="job-artifacts-delete">
<SvgIcon name="octicon-trash" class="ui text black job-artifacts-icon"/>
<a v-if="run.canDeleteArtifact" @click="deleteArtifact(artifact.name)" class="left-side-section-delete">
<SvgIcon name="octicon-trash" class="ui text black left-side-section-icon"/>
</a>
</li>
</ul>
</div>
<div class="left-side-section" v-if="run.workflowFileLink">
<div class="left-side-section-title">
{{ locale.runDetails }}
</div>
<ul class="left-side-section-list">
<li class="left-side-section-item">
<a class="left-side-section-link" target="_blank" :href="run.workflowFileLink">
<SvgIcon name="octicon-file" class="ui text black left-side-section-icon"/>{{ locale.workflowFile }}
</a>
</li>
</ul>
Expand Down Expand Up @@ -565,26 +580,26 @@ export function initRepositoryActionView() {
}
}

.job-artifacts-title {
.left-side-section-title {
font-size: 18px;
margin-top: 16px;
padding: 16px 10px 0 20px;
border-top: 1px solid var(--color-secondary);
}

.job-artifacts-item {
.left-side-section-item {
margin: 5px 0;
padding: 6px;
display: flex;
justify-content: space-between;
}

.job-artifacts-list {
.left-side-section-list {
padding-left: 12px;
list-style: none;
}

.job-artifacts-icon {
.left-side-section-icon {
padding-right: 3px;
}

Expand Down