Add `cache-dir` input to allow specifying cache content location during auto-discovery #52

omus · 2025-05-27T19:39:40Z

Follow up to #49. We need to be able to specify the location on the host system where the bind directory is located which will be used for injecting/extracting cache data. Without this change users can easily run into issues when using absolute paths for their id:

No cache map provided. Trying to parse the Dockerfile to find the cache mount instructions...
Cache map parsed from Dockerfile: {"/var/cache/apt":{"id":"/var/cache/apt","target":"/var/cache-target"},"/var/lib/apt":{"id":"/var/lib/apt","target":"/var/cache-target"}}
Error: EACCES: permission denied, open '/var/cache/apt/buildstamp'
    at async open (node:internal/fs/promises:639:25)
    at async Object.writeFile (node:internal/fs/promises:1216:14)
    at async $bd1d73aff0732146$var$injectCache (file:///home/runner/work/_actions/reproducible-containers/buildkit-cache-dance/653a570f730e3b9460adc576db523788ba59a0d7/dist/index.js:7152:5)
    at async $bd1d73aff0732146$export$38c65e9f06d3d433 (file:///home/runner/work/_actions/reproducible-containers/buildkit-cache-dance/653a570f730e3b9460adc576db523788ba59a0d7/dist/index.js:7199:72)
    at async $bec5d2ddaaf4a876$var$main (file:///home/runner/work/_actions/reproducible-containers/buildkit-cache-dance/653a570f730e3b9460adc576db523788ba59a0d7/dist/index.js:7306:9)
    at async file:///home/runner/work/_actions/reproducible-containers/buildkit-cache-dance/653a570f730e3b9460adc576db523788ba59a0d7/dist/index.js:7310:5 {
  errno: -13,
  code: 'EACCES',
  syscall: 'open',
  path: '/var/cache/apt/buildstamp'
}
Error: EACCES: permission denied, open '/var/cache/apt/buildstamp'
    at async open (node:internal/fs/promises:639:25)
    at async Object.writeFile (node:internal/fs/promises:1216:14)
    at async $bd1d73aff0732146$var$injectCache (file:///home/runner/work/_actions/reproducible-containers/buildkit-cache-dance/653a570f730e3b9460adc576db523788ba59a0d7/dist/index.js:7152:5)
    at async $bd1d73aff0732146$export$38c65e9f06d3d433 (file:///home/runner/work/_actions/reproducible-containers/buildkit-cache-dance/653a570f730e3b9460adc576db523788ba59a0d7/dist/index.js:7199:72)
    at async $bec5d2ddaaf4a876$var$main (file:///home/runner/work/_actions/reproducible-containers/buildkit-cache-dance/653a570f730e3b9460adc576db523788ba59a0d7/dist/index.js:7306:9)
    at async file:///home/runner/work/_actions/reproducible-containers/buildkit-cache-dance/653a570f730e3b9460adc576db523788ba59a0d7/dist/index.js:7310:5

omus · 2025-05-27T19:55:04Z

src/opts.ts

        const target = "/var/cache-target";

-        cacheMap[id] = {
+        cacheMap[`${bindRoot}/${id}`] = {


A breaking change because of this. I could update this to allow bindRoot to be null to restore the original behavior. However, the only scenario where the original behavior could work reliably is when the ids were all explicitly set which would make their source be the working directory. We could avoid breakage by making the default cache-dir be . if we want a mostly breaking change.

I really like the approach of maintaining backward compatibility by defaulting cache-dir to ..

This way, we avoid having to release a major version with breaking changes.

I'll update this to avoid having a breaking change

Changes are now fully backwards compatible. I didn't go with the cache-dir="." approach as that was still breaking for cache mount ids with absolute paths

omus · 2025-05-27T20:30:22Z

@bennesp did you want to give this a review?

bennesp

I understand the problem you’re addressing and I really appreciate the solution you’ve proposed.

At the same time, I would prefer if the change could be implemented in a way that avoids breaking compatibility, so it doesn’t break any existing workflows.

That said, since I’m not the repository owner, this is just my opinion, feel free to challenge it 😄

bennesp · 2025-05-29T07:24:26Z

README.md

        with:
          images: Build

      - name: Cache


Switching this example from two caches to just one sounds a bit confusing.

In my use cases it’s important to use two different caches because they have different lifecycles: one gets reused more often, while the other is invalidated more frequently.

So it would be nice to keep an example showing how to handle both.

Can you elaborate more on what you'd like to see? The updated example uses one GHA cache for all of the cache mounts just like the original example did. The only difference here is that the cache-map is generated from the Dockerfile and that the locations of the cache mounts on disk are ./cache-mount//var/cache/apt and ./cache-mount//var/lib/apt instead of ./var-cache-apt and ./var-lib-apt.

Oh I think I got confused by the two paths we had previously, but indeed the cache is just one.

To be sure I got it right, please follow this scenario:

A Dockerfile with two caches: --mount=type=cache,id=var-cache-apt and --mount=type=cache,id=var-lib-apt

A workflow like the following:

- name: Cache uses: actions/cache@v3 id: cache with: path: | var-cache-apt var-lib-apt key: cache-${{ hashFiles('.github/workflows/test/Dockerfile') }} - name: Restore Docker cache mounts uses: reproducible-containers/[email protected]

Given the two points above, I would expect the "Restore" step to work correctly, by populating the builder cache with the content of ./var-cache-apt and ./var-lib-apt.

Is it correct?

Given the Dockerfile you mentioned the example you provided is correct given that a cache-map is also provided. An example with that included would be:

- name: Cache uses: actions/cache@v3 id: cache with: path: | var-cache-apt var-lib-apt key: cache-${{ hashFiles('.github/workflows/test/Dockerfile') }} - name: Restore Docker cache mounts uses: reproducible-containers/[email protected] with: cache-map: | { "var-cache-apt": "/var/cache/apt", "var-lib-apt": "/var/lib/apt" }

Alternatively these equivalent examples would also work with that same Dockerfile with variations to the cache-map:

Extracting the cache-map from the Dockerfile

- name: Cache uses: actions/cache@v3 id: cache with: path: | /var/cache/apt /var/lib/apt key: cache-${{ hashFiles('.github/workflows/test/Dockerfile') }} - name: Restore Docker cache mounts uses: reproducible-containers/[email protected] with: dockerfile: Dockerfile

Note: This example would only be possible if additional permissions are granted in GHA. I tried this and ran into this problem:

No cache map provided. Trying to parse the Dockerfile to find the cache mount instructions... Cache map parsed from Dockerfile: {"/var/cache/apt":{"id":"/var/cache/apt","target":"/var/cache-target"},"/var/lib/apt":{"id":"/var/lib/apt","target":"/var/cache-target"}} Error: EACCES: permission denied, open '/var/cache/apt/buildstamp' at async open (node:internal/fs/promises:639:25) at async Object.writeFile (node:internal/fs/promises:1216:14) at async $bd1d73aff0732146$var$injectCache (file:///home/runner/work/_actions/reproducible-containers/buildkit-cache-dance/653a570f730e3b9460adc576db523788ba59a0d7/dist/index.js:7152:5) at async $bd1d73aff0732146$export$38c65e9f06d3d433 (file:///home/runner/work/_actions/reproducible-containers/buildkit-cache-dance/653a570f730e3b9460adc576db523788ba59a0d7/dist/index.js:7199:72) at async $bec5d2ddaaf4a876$var$main (file:///home/runner/work/_actions/reproducible-containers/buildkit-cache-dance/653a570f730e3b9460adc576db523788ba59a0d7/dist/index.js:7306:9) at async file:///home/runner/work/_actions/reproducible-containers/buildkit-cache-dance/653a570f730e3b9460adc576db523788ba59a0d7/dist/index.js:7310:5 { errno: -13, code: 'EACCES', syscall: 'open', path: '/var/cache/apt/buildstamp' }

Extracting the cache-map from the Dockerfile and use cache-dir

- name: Cache uses: actions/cache@v3 id: cache with: path: | cache-mount/var/cache/apt cache-mount/var/lib/apt key: cache-${{ hashFiles('.github/workflows/test/Dockerfile') }} - name: Restore Docker cache mounts uses: reproducible-containers/[email protected] with: dockerfile: Dockerfile cache-dir: cache-mount

This would generate the following cache-map:

{ "cache-mount//var/cache/apt": { "id":"/var/cache/apt", "target":"/var/cache-target" }, "cache-mount//var/lib/apt":{ "id":"/var/lib/apt", "target":"/var/cache-target" } }

This example is equivalent to what I currently am proposing just a little more verbose for the path in actions/cache.

bennesp · 2025-05-29T07:29:30Z

src/opts.ts

        const target = "/var/cache-target";

-        cacheMap[id] = {
+        cacheMap[`${bindRoot}/${id}`] = {


I really like the approach of maintaining backward compatibility by defaulting cache-dir to ..

This way, we avoid having to release a major version with breaking changes.

action.yml

AkihiroSuda · 2025-05-31T16:16:15Z

README.md

+              "var-lib-apt": "/var/lib/apt"
+            }
+          skip-extraction: ${{ steps.cache.outputs.cache-hit }}
+```


How is this change relevant to cache-dir?

As I updated the primary example of how to use this action above to demonstrate how to use dockerfile/cache-dir I moved the snippet from the original example here. Doing this was necessary as this sentence no longer made sense since there was no example using a "single string":

Optionally, instead of a single string for the target you can provide an object with additional options that should be passed to --mount=type=cache in the values cache-map JSON. The target path must be present in the object as a property.

AkihiroSuda · 2025-05-31T16:17:50Z

Please squash commits that are related to the cache-dir.
(Changes irrelevant to cache-dir should be separate commits, or ideally separate PRs)

Signed-off-by: Curtis Vogt <[email protected]>

omus · 2025-06-02T13:48:03Z

Please squash commits that are related to the cache-dir. (Changes irrelevant to cache-dir should be separate commits, or ideally separate PRs)

Done. I've kept the README change as a separate commit as it appears you may want that removed from this PR entirely

omus · 2025-06-13T15:45:52Z

@AkihiroSuda can you give this another review?

AkihiroSuda · 2025-06-15T16:49:06Z

Will review this week, sorry for the delay

AkihiroSuda

Thanks, sorry for the delay

omus force-pushed the cv/cache-dir branch from d1cf60f to ee35d46 Compare May 27, 2025 19:50

omus mentioned this pull request May 27, 2025

Generate cache-map from Dockerfile beacon-biosignals/docker-build#30

Merged

omus commented May 27, 2025

View reviewed changes

omus marked this pull request as ready for review May 27, 2025 19:55

omus force-pushed the cv/cache-dir branch from c1019f6 to 0895856 Compare May 27, 2025 20:09

bennesp suggested changes May 29, 2025

View reviewed changes

omus force-pushed the cv/cache-dir branch from 8ceeab1 to 6afbe2f Compare May 30, 2025 18:02

omus requested a review from bennesp May 30, 2025 18:46

AkihiroSuda reviewed May 31, 2025

View reviewed changes

action.yml Show resolved Hide resolved

AkihiroSuda reviewed May 31, 2025

View reviewed changes

omus force-pushed the cv/cache-dir branch from 2b4f808 to 40ef99c Compare June 2, 2025 13:41

omus added 2 commits June 2, 2025 08:45

Allow users to specify cache-dir location used in auto-discovery

4e6d40f

Signed-off-by: Curtis Vogt <[email protected]>

Provide dockerfile/cache-dir example in README

ee00176

Signed-off-by: Curtis Vogt <[email protected]>

omus force-pushed the cv/cache-dir branch from 1d3f699 to ee00176 Compare June 2, 2025 13:46

omus requested a review from AkihiroSuda June 2, 2025 13:48

AkihiroSuda approved these changes Jun 20, 2025

View reviewed changes

AkihiroSuda merged commit 5b81f4d into reproducible-containers:main Jun 20, 2025
3 checks passed

Add cache-dir input to allow specifying cache content location during auto-discovery #52

Add cache-dir input to allow specifying cache content location during auto-discovery #52

Uh oh!

Conversation

omus commented May 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

omus May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

omus commented May 27, 2025

Uh oh!

bennesp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda commented May 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

omus commented Jun 2, 2025

Uh oh!

omus commented Jun 13, 2025

Uh oh!

AkihiroSuda commented Jun 15, 2025

Uh oh!

AkihiroSuda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add `cache-dir` input to allow specifying cache content location during auto-discovery #52

Add `cache-dir` input to allow specifying cache content location during auto-discovery #52

omus May 30, 2025 •

edited

Loading

AkihiroSuda commented May 31, 2025 •

edited

Loading