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

Close #302 Preserve mtime when copying to/from the cache #336

Closed
wants to merge 11 commits into from

Conversation

schneems
Copy link
Contributor

Commits have more details. The short version is that I confirmed in #302 (comment) that mtime behavior is preserved with the CNB cache. However, the fs_extra method of copying to/from the cache did not preserve mtime. The cp_r library is built to handle this case.

When copying from the cache back into the app, it should not replace files that already exist. This test asserts already passing behavior.
Adds tests for ensuring FIFO behavior in the cache works based on mime types. This comment #302 (comment).

Currently it works locally but fails on CI, that tells me the mtime-preserving behavior is platform dependent.
It appears the behavior of fs-extra with regard to mtime for copying/moving directories is system dependent. To compensate, I switched to `cp_r` which explicitly is designed to preserve mtime https://crates.io/crates/cp_r. It has ~27,000 lifetime downloads and has activity as recently as a 9 days ago. https://docs.rs/cp_r/latest/cp_r/#release-history

Copying and preserving mtime is the primary reason this library exists. From the docs.rs:

> Copy a directory tree, including mtimes and permissions.

It does not have the ability to skip-overwriting existing files which I need, however it has a generic `filter` closure which allows me to assemble this behavior myself.
```
- Rake assets install
  - Detected rake (`rake` gem found, `Rakefile` found at `/workspace/Rakefile`)
  - Running `bundle exec rake -P --trace` .... (1.6s)
  - Compiling assets with cache (detected `rake assets:precompile` and `rake assets:clean` via `bundle exec rake -P`)
  - Loading cache for /workspace/public/assets
  - Loading cache for /workspace/tmp/cache/assets
  - Running `bundle exec rake assets:precompile assets:clean --trace`

      ** Invoke assets:precompile (first_time)
      ** Invoke assets:environment (first_time)
      ** Execute assets:environment
      ** Invoke environment (first_time)
      ** Execute environment
      ** Execute assets:precompile
      ** Invoke assets:clean (first_time)
      ** Invoke assets:environment 
      ** Execute assets:clean

  - Done (1.1s)
  - Storing cache for /workspace/public/assets
  - Storing cache for /workspace/tmp/cache/assets
- Debug info
  - Could not move files out of the application to the cache.
    From: /workspace/tmp/cache/assets To: /layers/heroku_ruby/cache_tmp_cache_assets
    Error: creating directory: /layers/heroku_ruby/cache_tmp_cache_assets/sprockets: File exists (os error 17)

! Error caching frontend assets
!
! An error occurred while attempting to cache frontend assets, and the Ruby buildpack
! cannot continue.
!
! Ensure that the permissions on the files in your application directory are correct and that
! all symlinks correctly resolve.

ERROR: failed to build: exit status 1
ERROR: failed to build: executing lifecycle: failed with status code: 51
Error: Process completed with exit code 1.
0s
```
@schneems
Copy link
Contributor Author

Error on second run with the cache:

- Debug info
  - Could not move files out of the application to the cache.
    From: /workspace/tmp/cache/assets To: /layers/heroku_ruby/cache_tmp_cache_assets
    Error: creating directory: /layers/heroku_ruby/cache_tmp_cache_assets/sprockets: File exists (os error 17)

! Error caching frontend assets
!
! An error occurred while attempting to cache frontend assets, and the Ruby buildpack
! cannot continue.
!
! Ensure that the permissions on the files in your application directory are correct and that
! all symlinks correctly resolve.

ERROR: failed to build: exit status 1
ERROR: failed to build: executing lifecycle: failed with status code: 51

Is due to cp_r. I misread the docs it says:

Overwrite existing directories or files.

However, it's under the banner "Missing features that could be added". It seems easy enough to implement following fs_err's API https://docs.rs/fs_extra/1.3.0/fs_extra/dir/struct.CopyOptions.html.

Alternatively we could have an on_conflict API that took a Fn(&Path, &Path) -> Result<&Path, Error> where it passes in the path to both the from and to paths and lets you return the path to the one you want to keep (or optionally raise an error. It's a little more verbose, but would allow for all possible combinations with a single API.

@schneems
Copy link
Contributor Author

Closing in favor of #337

@schneems schneems closed this Oct 17, 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.

1 participant