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

Does cache work? #142

Closed
vixalien opened this issue Jun 13, 2023 · 30 comments
Closed

Does cache work? #142

vixalien opened this issue Jun 13, 2023 · 30 comments

Comments

@vixalien
Copy link
Contributor

I'm trying to setup CI for github.com/vixalien/muzika

but it seems like the cache doesn't work correctly and GTK ends up being recompiled again.. Does one need to use https://github.com/marketplace/actions/cache?

Here's my workflow file:

name: CI

on:
  push:
    branches: [main]
  pull_request:
    branches: [main]

permissions:
  contents: read

jobs:
  flatpak:
    name: "Flatpak"
    runs-on: ubuntu-latest
    container:
      image: bilelmoussaoui/flatpak-github-actions:gnome-44
      options: --privileged
    steps:
      - uses: actions/checkout@v3
        with:
          persist-credentials: false
      - uses: flatpak/flatpak-github-actions/flatpak-builder@v6.1
        with:
          bundle: muzika.flatpak
          manifest-path: build-aux/flatpak/com.vixalien.muzika.json
          cache: true
          restore-cache: true
          cache-key: flatpak-builder-${{ github.workflow_sha }}
@bilelmoussaoui
Copy link
Member

how do you check if the cache is working or not?

@vixalien
Copy link
Contributor Author

By running the same workflow multiple times, and then checking it runs from scratch or if skips compiling unchanged dependencies.

@vixalien
Copy link
Contributor Author

see https://github.com/vixalien/muzika/actions for the runs

@bilelmoussaoui
Copy link
Member

Rerunning the workflow won't make the caching work, you have to push a new commit

@vixalien
Copy link
Contributor Author

I know.

which is why I'm pushing new commits to test, to no avail.

see vixalien/muzika#13

@andyholmes
Copy link
Contributor

It looks like it's working: https://github.com/vixalien/muzika/actions/caches

@vixalien
Copy link
Contributor Author

It's caching yes, but it doesn't seem to be restoring the cache..

@vixalien
Copy link
Contributor Author

the build logs say No cache was found. maybe I'm using a wrong cache-key

@andyholmes
Copy link
Contributor

I don't recall exactly how this action organizes caches, but they may be tied to the PR branch. I would recommend you get a build running on your main branch, then see if a PR will pick-up the cache.

You should also know that flatpak-builder generally follows a linear path from top-to-bottom of a manifest. For example, if you change the sassc module, it will rebuild GTK4 and everything "after" it. All GitHub is doing is taking flatpak-builder's cache folder and stuffing it somewhere on its servers.

Incidentally, you probably want to move libadwaita below gtk, since you're likely building it against the version of GTK4 in the platform, rather than the commit you've chosen.

@vixalien
Copy link
Contributor Author

alright! I'll merge the PR and report back when the main branch has triggered some CI builds.

You should also know that flatpak-builder generally follows a linear path from top-to-bottom of a manifest

While I'm not sure about what that means exactly, I think I've set it up correctly...

This is because there's a libadwaita module which has other modules as children (gtk, sassc etc..) Do you know how I could fix it?

@andyholmes
Copy link
Contributor

Just think of it like a project, building each dependency before the module that depends on it. So you want:

  • libsass
  • sassc
  • gtk
  • libadwaita

Because libadwaita depends on gtk, which depends on sassc, and so on.

@vixalien
Copy link
Contributor Author

I've had a couple of CI runs, and still the cache doesn't seem to be working

@andyholmes
Copy link
Contributor

andyholmes commented Jun 29, 2023

It seems like your cache key is changing, even between runs. In #48 it was flatpak-builder-0be6971d38629b561d98a17bc24a450da53c571c, but in #47 it was flatpak-builder-d96b7c7c6eee5b61f251c056654bae5f10444e38.

It looks like you are using github.workflow_sha for cache-key. Maybe try using the default, which is a hash of the manifest being built.

Also note your most recent change modified the libadwaita module at the top of the manifest, so that would have resulted in re-building most of the Flatpak anyways.

@vixalien
Copy link
Contributor Author

by "default cache-key" do you mean that I should omit it, or set it to cache-key: flatpak-builder-${{ github.sha }} as per the docs.

vixalien added a commit to vixalien/muzika that referenced this issue Jun 29, 2023
@andyholmes
Copy link
Contributor

By "default" I mean don't set it all, and let the GitHub action pick it for you. It will use a hash of the flatpak manifest, and only create a new cache when that file changes.

@vixalien
Copy link
Contributor Author

alright. I've implemented your recommendations and I'll update after a few CI runs

@vixalien
Copy link
Contributor Author

I removed the cache-key, and the behavior still persists...

@andyholmes
Copy link
Contributor

Both these runs (#51 and #50) seem to be trying save with the same key (some editing done for readability):

Failed to save: Unable to reserve cache with key flatpak-builder-f65e114936d553b0aaa0-x86_64,
another job may be creating this cache. More details: Cache already exists.
  Scope: refs/heads/main,
  Key: flatpak-builder-f65e114936d553b0aaa0-x86_64,
  Version: cc445e94901a613836a562c602315a9af315d6c9fcd330f5eb77754459881d24

This implies that the cache is being saved, but not restored for some reason. I believe the code is correct from what I can see (here for reference).

I'd say you might have found a confirmed bug, but I'm not sure because it doesn't print the cache key or the cache hit if it fails. Sorry, I've reached the end of my best guesses what's happening here :/

@vixalien
Copy link
Contributor Author

I see that this can happen when cache fails to be committed. will update this after a few other CI runs to see if it's indeed a bug

@vixalien
Copy link
Contributor Author

vixalien commented Jul 2, 2023

okay I found the culprint. for some reason I had the following in the workflow file.

permissions:
  contents: read

I reset the file using the template from the page and now I can see the cache gets saved correctly.

However, it doesn't get restored still.

I omitted the cache-key but I think that's what causing it??? The default is flatpak-builder-${sha256(manifestPath)}, however when I type that manually, it doesn't subsitute the variables in braces so I omitted it.

Also interesting is that all the examples on the github marketplace page use flatpak-builder-${{ github.sha }} as cache-key. Should I use that?

@vixalien
Copy link
Contributor Author

vixalien commented Jul 5, 2023

solved by using flatpak/flatpak-github-actions/flatpak-builder@4 (instead of @6.1)

and cache-key: flatpak-builder-${{ github.sha }}

(I'm not sure which one of the two solved it, but I'm too tired to find out)

@vixalien vixalien closed this as completed Jul 5, 2023
@vixalien
Copy link
Contributor Author

I can confirm this only happens when you use @v6.1.

I'm reopening this since it's indeed an issue that prevents upgrading. I can only confirm it works on @v4

@vixalien vixalien reopened this Jul 15, 2023
@xfangfang
Copy link
Contributor

xfangfang commented Jul 23, 2023

I encountered the same problem, and in order to avoid the impact of restore-keys, I set the key to: wiliwili-flatpak-${{ matrix.driver }}-${{ hashFiles('.flatpak-manifest.yml') }}(Note that I have modified the prefix to ensure accurate matching at all times.)

Run on default branch:

  1. The first time, the cache was successfully created: wiliwili-flatpak-gl-3e4c6feaf306aeb897d55c1602e3a05ca5c95d87f46f1087b2c608f2b302b66b-x86_64
  2. The second time, the cache missing, but after compilation, when saving the cache, it was displayed:
Failed to save: Unable to reserve cache with key wiliwili-flatpak-gl-3e4c6feaf306aeb897d55c1602e3a05ca5c95d87f46f1087b2c608f2b302b66b-x86_64, another job may be creating this cache. More details: Cache already exists. Scope: refs/heads/yoga, Key: wiliwili-flatpak-gl-3e4c6feaf306aeb897d55c1602e3a05ca5c95d87f46f1087b2c608f2b302b66b-x86_64, Version: cc445e94901a613836a562c602315a9af315d6c9fcd330f5eb77754459881d24
  1. The third time, using another workflow actions/cache with the same key and the same cache path(.flatpak-builder) on the same branch, the cache missing, I casually wrote something to the cache path, and a cache with the same key was created after the run ended.
  2. The fourth time, executed main workflow, and the cache hit, but it hit the cache created in the third time.

@vixalien
Copy link
Contributor Author

so your workaround will only start working from the 4th run?

@xfangfang
Copy link
Contributor

@vixalien That's not a solution, it's the step I tried to reproduce the problem. You will find that the incorrect cache content was restored for the fourth time. Based on the above operation, I guess the problem may have occurred during the upload cache.

I am now using a simpler way to make the cache effective,set cache to false in flatpak-github-actions/flatpak-builder,and manually add a cache action before it。

  - name: Cache
    id: cache
    uses: actions/cache@v3
    with:
      path: .flatpak-builder
      key: wiliwili-flatpak-${{ matrix.driver }}-${{ hashFiles('.flatpak-manifest.yml') }}-${{ matrix.arch }}

For my project, the cache size has decreased from 350M to 140M, but it has no impact on compilation, and the cache has also taken effect.

mattiasegly added a commit to mattiasegly/flatpak-kadas that referenced this issue Jul 23, 2023
@vixalien
Copy link
Contributor Author

but it has no impact on compilation

does this mean cache works correctly? as in does the compiler use the cache?

@xfangfang
Copy link
Contributor

Here is the log:

Starting build of cn.xfangfang.wiliwili
Cache hit for libv4l2, skipping build
Cache hit for nv-codec-headers, skipping build
Cache hit for ffmpeg, skipping build
Cache hit for libass, skipping build
Cache hit for uchardet, skipping build
Cache hit for libmpv, skipping build
Cache hit for libwebp, skipping build
Cache miss, checking out last cache hit
========================================================================
Building module wiliwili in /__w/wiliwili/wiliwili/.flatpak-builder/build/wiliwili-1

When cache hits, no dependencies need to be recompiled.

I haven't paid much attention to the behavior of caching before. It seems that the original cache not only avoids the compilation of dependencies, but also reduces the compilation of the main program.

And manually using cache action will completely recompile the main program every time, because my program compiles slowly, so I don't want to spend time on it anymore. Manually using cache action is already enough for me to use.

@xfangfang
Copy link
Contributor

@vixalien

I figured this out, The reason for this issue is an error in the upstream. It should be resolved when this PR is merged:
: actions/toolkit#1378 (comment)

Before that, you can temporarily use:xfangfang/flatpak-github-actions/flatpak-builder@master

@bilelmoussaoui
Copy link
Member

This was resolved thanks to @xfangfang. I will roll out a release some time this week and backport the fixes

@vixalien
Copy link
Contributor Author

I completely missed this. Thanks everyone!!

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

No branches or pull requests

4 participants