Skip to content

Conversation

ChristopherHX
Copy link
Contributor

Use a helper method around the jobparser for parsing a single job structure from an ActionRunJob

@ChristopherHX ChristopherHX added type/refactoring Existing code has been cleaned up. There should be no new functionality. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/gitea-actions related to the actions of Gitea labels Oct 10, 2025
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 10, 2025
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Oct 10, 2025
@wxiaoguang
Copy link
Contributor

Made some changes in 0c14f30 .

Since parsedWorkflows[0].Job() can return nil, I think we should treat such case as error

if wfJobs, _ := jobparser.Parse(actionRunJob.WorkflowPayload); len(wfJobs) == 1 {
_, wfJob := wfJobs[0].Job()
hasIf = len(wfJob.If.Value) > 0
// FIXME evaluate this on the server side
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you elaborate more details? For example, what wrong would happen and how to evaluate on server side?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I plan to create a more detailed refactoring proposal issue collection soon. 1

how to evaluate on server side?

This is similar to what concurrency for jobs is currently doing, Gitea references gitea/act that has all code for doing this kind of thing.

what wrong would happen

  • a to be skipped job lands in the concurrency group and prevents a non skipped job of the same group from running
  • a to be skipped job waits for a runner to become online to update the job status to skipped
    • gitea can just update the status to skipped without additional db writes, by skipping the Status Waiting.
  • all contexts should be available to the server
    • needs
    • vars
    • github/gitea
    • (env) extension

More refactoring is needed for those examples:

  • strategy (matrix) is evaluated even if the condition is false (extension matrix in if then strategy must evaluate earlier)
    • e.g. the expression in strategy depends on outputs not being set or you want to skip all
  • Reusable Workflows without runs-on should run on server as well and they should be able to deliver jobs to multiple different runners
  • jobif: ${{ cancelled() }} should be able to ignore cancel requests to be delivered to the runner
    • currently gitea force cancels every job, without a grace period of 5min

Footnotes

  1. The problems I describe are platform differences between GitHub and Gitea Actions that I would like to remove, already existing enhancements towards Actions should be unaffected

@ChristopherHX
Copy link
Contributor Author

Since parsedWorkflows[0].Job() can return nil, I think we should treat such case as error

Sounds good, would improve logging

@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 Oct 11, 2025
@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 Oct 11, 2025
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 11, 2025
@lunny lunny merged commit 25c4eb1 into go-gitea:main Oct 11, 2025
26 checks passed
@GiteaBot GiteaBot added this to the 1.26.0 milestone Oct 11, 2025
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 11, 2025
zjjhot added a commit to zjjhot/gitea that referenced this pull request Oct 13, 2025
* giteaofficial/main:
  [skip ci] Updated translations via Crowdin
  Update JS deps, misc tweaks (go-gitea#35643)
  Bump actions/checkout to v5 (go-gitea#35644)
  nix flake update (go-gitea#35639)
  Cleanup ActionRun creation (go-gitea#35624)
  bump archives&rar dep (go-gitea#35637)
  Fix merge panic (go-gitea#35606)
  Bump happy-dom from 19.0.2 to 20.0.0 (go-gitea#35625)
  Refactor ActionRunJob parsing into a reusable function (go-gitea#35623)
  Fix code tag style problem and LFS view bug (go-gitea#35628)
  Support Actions `concurrency` syntax (go-gitea#32751)
  The status icon of the Action step is consistent with GitHub (go-gitea#35618)
  Mock external service in hcaptcha TestCaptcha (go-gitea#35604)
  Fix inputing review comment will remove reviewer (go-gitea#35591)
  [skip ci] Updated translations via Crowdin
  Fix diffpatch API endpoint (go-gitea#35610)
  Print PR-Title into tooltip for actions (go-gitea#35579)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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 skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/gitea-actions related to the actions of Gitea 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