Skip to content
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

Improve ci cd workflow #614

Closed
wants to merge 24 commits into from
Closed

Conversation

chippmann
Copy link
Contributor

@chippmann chippmann commented Apr 18, 2024

This PR contains the following changes:

  • hopefully improves our cache usage by only using the cache name and no refs for cache creation
  • always prints the testoutput now (including the error out)
  • Adds a workflow which builds and tests on every commit for linux only but does not (fully) run on pull requests

CedNaru and others added 7 commits April 5, 2024 14:04
* Move files to new jvm_wrapper directory
* Refactor JVM wrappers
* Transform many classes into JvmSingletonWrapper
* Merge MemoryManager and MemoryBridge
---------

Co-authored-by: Pierre-Thomas Meisels <meisels27@yahoo.fr>
* Rework JVM configuration parsing and checks
* Add jvm_arguments options

---------

Co-authored-by: LukasVykuka <LukasVykuka@users.noreply.github.com>
* Fix android export templates

* Test cherry pick

* Delete unneded workflows

* Use different commit

* Add identity

* Fix paths

* Fix version in export templates

* Add todo

* Add deleted workflows again

* Add temporary workaround to other pipelines

* Disable existing pipelines and start on pr_pipeline with linux

* Use environment prefix

* Fix scons flags

* Pass godot version as an input instead and add jvm build workflow

* Hardcode input values as test

* Fix input reference

* Fix input usage

* Remove matrix build and fix concurrency

* Remove usless steps

* Add windows build

* Define version at job definition level as github does not support those to be shared

* Change windows emoji

* Add macos builds

* Change jvm emoji

* Upgrade checkout v2 to checkout v4

* Use `gradle-executable` instead of deprecated `wrapper-directory`

* Replace deprecated eskator gradle build with gradle setup and plain gradle execution

* Remove jvm emoji on job name

* Use workflow prefixes instead of suffixes

* Define jvm distribution

* Add android and ios builds

* Add linux tests

* Add windows script to run unit tests

* Add assemble macos pipeline

* Fix referenced workflow

* Change job name

* Change job names

* Add assemble linux editor and improve naming on assemlbe macos editor

* Fix path

* Fix paths and add other assemble tasks

* Fix emojis

* Always use cache

* Downgrade upload and download actions to use the same version as godot

* Also build debug versions

* Fix debug build

* Prefix artifact names

* Fix emojis

* Remove explicit matrix name definition

* Rename matrix names

* Add windows and macos tests

* Cleanup runs-on definitions

* Add assemble ios and export templates tpz

* Fix needs

* Fix names

* Fix names

* Fix names

* Fix needs declaration for tpz

* Remove unneded needs

* Fixes

* Add fake step

* Test

* Add label and comment

* Change type

* Fix dependencies and test trigger on windows

* Set test timeout eagerly

* Fix output reference

* Fixes

* Add deploy stages

* Fix bat

* Fix emoji

* Try wrapping in ""

* Change name

* Use the correct target

* Fix name path swapped

* Add needed inputs

* Add missing inputs

* Set java version

* Fix version reference

* Fix naming

* Remove predefined outputs

* Update shell script

* Update zip commands

* Don't error out on error logs in test execution

* Fix command to execute

* Run commands synchronously

* Enable echo to fix failing test because of it

* Improve script

* Set echo off

* Fix assemble of linux editor

* test

* Replace test execution scripts with gradle task

* Find editor executable in gradle task

* Remove lines giving execution rights to non existent script

* Print files

* Use delegate access

* Revert

* Fix print to debug issue

* Remove additional filters

* Check the name explicitly

* Try passing params as one

* Increase test timeouut to 30 minutes

* Test

* Ignore exit code

* Properly ignore exit value

* Properly ignore exit value

* Fix string occurrence checks

* Rewrite output parsing

* Rewrite output parsing

* Use file output stream as standardOutput

* Print output if assetions failes

* Fix println call

* tmp

* Disable long string test for now

* Fix paths

* Fix some names and align tag variant

* Delete old workflows and add comment on secrets inherit

* REVERT_ME: temporarily comment out jvm artifact deployment for deployment testing

* Add docs deployment

* Fix workflow name

* Add macos editor to deployments

* Add description on editor flavours

* Delete obsolete actions

* Add more comments

* Change download action

* Readd action which is used

* Build tests project once and use that for the tests

* Rename workflows

* Fix upload url

* Add missing depends

* Revert "Build tests project once and use that for the tests"

This reverts commit 8fd3658.

* Fix export templates

* Revert "REVERT_ME: temporarily comment out jvm artifact deployment for deployment testing"

This reverts commit f33e0f7.

* Only remove debug symbols on release builds

* Add documentations on the debug and release builds of the editor

* Fix indents

* Make the info block a warning instead

* Align documentation with message in github release

* Fix creation of embedded jvm

* Release actual debug builds and use dev builds for tests

* Fix bootstrap download in tests

* Update gradle version

* Bump intellij plugin version to cope with new gradle version
@chippmann chippmann changed the base branch from master to gd_kotlin_rework April 18, 2024 07:41
@chippmann chippmann marked this pull request as ready for review April 18, 2024 08:46
@CedNaru CedNaru deleted the branch master April 21, 2024 06:10
@CedNaru CedNaru closed this Apr 21, 2024
@CedNaru CedNaru reopened this Apr 21, 2024
@CedNaru
Copy link
Member

CedNaru commented Apr 24, 2024

I don't think this is the right approach to save on cache space. I have read the documentation in details.
Caches are created per key AND per branch and can only be accessed by that branch or a child branch.

Let's take this PR as en example. This branch is a child of gd_kotlin_rework, itself a child of master
If you try to fetch a cache here, it's going to check for the key in this branch's cache, then in gd_kotlin_rework then in master.
Here is the relevant part in the doc:
image

But afterward, a new cache will be saved automatically by the action, a cache that will only be accessible from this PR.
It means that none of the work cached in that branch will be useable by the others.
The way I see to properly share cache between branches is to only save the cache when running on the master and prevent saving on other branches. Or at least to only save cache on merge actions, that way we only generate caches for "parent branches" whatever they are (in this case, that would be cached for gd_kotlin_rework).
Preventing generating cache on "leaf" branches is important so we don't bloat the cache with each PR:
image

So you should keep your different keys for all kind of build (release, debug and dev should all be a separate key).
And instead only save when the conditions are met (either you are on master or triggered by a merge action if that's possible) so the amount of generated cache is kept to a minimum.
To be more granular with restoring and saving caches, you can use the actions/cache/restore and actions/cache/save instead of the more generic actions/cache that both restore and save automatically.

As it is, I don't think your PR is going to save on cache, and on top will force a different build to restore a cache they can't use because you made them all use the same key.

@CedNaru CedNaru force-pushed the gd_kotlin_rework branch 3 times, most recently from 8817a3f to 628389e Compare May 7, 2024 03:31
@CedNaru CedNaru force-pushed the gd_kotlin_rework branch 4 times, most recently from 7a8a805 to e2c6543 Compare May 9, 2024 13:10
Base automatically changed from gd_kotlin_rework to master May 9, 2024 16:03
@chippmann
Copy link
Contributor Author

Superseeded by #638

@chippmann chippmann closed this May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants