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

Preserving app file mtimes at build time #492

Closed
pclalv opened this issue Feb 10, 2020 · 6 comments · Fixed by #505
Closed

Preserving app file mtimes at build time #492

pclalv opened this issue Feb 10, 2020 · 6 comments · Fixed by #505
Assignees
Labels
good first issue A good first issue to get started with. size/sm Small level of effort type/enhancement Issue that requests a new feature or improvement.
Milestone

Comments

@pclalv
Copy link

pclalv commented Feb 10, 2020

As of lifecycle#80, the lifecycle ensures that all app
files have the same predetermined mtime at build time, regardless of
the app file's actual mtime. It would be nice to have an option to
preserve app file mtimes at buildtime in order to support build steps
that use mtime to validate the cache and/or its contents.

Detailed use-case

I have a Rails app with a Sprockets asset pipeline. In
order to build images for the Rails app we run several it through
several buildpacks, including a Ruby buildpack and a Sprockets asset
compilation buildpack.

The most basic Sprockets asset compilation buildpack essentially just
needs to compile static assets from source files and write those
compiled assets to a known place. It can compile every asset from
scratch every time and generate the correct results.

However, compiling every asset from scratch every time is not
efficient. We would like to enable our Sprockets buildpack to be able
to use a cache and thereby only compile assets that need to be
compiled. Sprockets is built with support for caching; as with GNU
make, a source file's mtime is the underlying mechanism for
determining whether a cached compiled asset is valid. Because every
source file has the same mtime on every build, a Sprockets buildpack
will compile an asset from a source file exactly once and never
compile it again, due to the source file mtime being the same every
every time.

@ekcasey
Copy link
Member

ekcasey commented Feb 10, 2020

@pclalv When exporting image layers we want to normalize all file times so that we can reuse existing blobs and avoid expensively re-uploading layers to the registry when the layer file content has not changed.

Fortunately, if the issue you are experiencing is caused by source file mtimes in the app dir during build, this is not incompatible with normalized times at export. It appears that pack is unnecessarily normalizing mtimes when copying the app from the filesystem into the build container. pack could preserve the mtimes during app copy allowing them to be set to real values while your buildpack runs. The exporter should still normalize all date times when creating the final image.

@zmackie
Copy link
Contributor

zmackie commented Feb 10, 2020

@ekcasey Am I right in thinking this is an issue in pack and not the lifecycle?

@ekcasey
Copy link
Member

ekcasey commented Feb 10, 2020

@zmackie I believe so, I'll move it over.

@ekcasey ekcasey transferred this issue from buildpacks/lifecycle Feb 10, 2020
@jromero jromero added the status/triage Issue or PR that requires contributor attention. label Feb 10, 2020
@pclalv
Copy link
Author

pclalv commented Feb 12, 2020

Ah yeah, my mistake. While we invoke the lifecycle directly in our remote builds, I usually test our buildpacks locally with pack, and I mistakenly assumed that lifecycle builds and pack builds would be equivalent in this regard. It would be nice to smooth out this difference, though.

@ameyer-pivotal
Copy link
Contributor

Hi @pclalv, thanks for reporting this! We're in agreement with @ekcasey in that we should be able to preserve source file mod times during the build with no negative side effects (they will still get normalized before exporting the final image). This will resolve your issue and also #184.

@ameyer-pivotal ameyer-pivotal added good first issue A good first issue to get started with. size/sm Small level of effort status/ready Issue ready to be worked on. type/enhancement Issue that requests a new feature or improvement. and removed status/triage Issue or PR that requires contributor attention. labels Feb 13, 2020
@ameyer-pivotal ameyer-pivotal self-assigned this Feb 18, 2020
@ameyer-pivotal ameyer-pivotal added status/in-progress and removed status/ready Issue ready to be worked on. labels Feb 18, 2020
@ameyer-pivotal
Copy link
Contributor

Hi @pclalv, we've got PR #505 up for this. Feel free to take a look at that branch and try it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue A good first issue to get started with. size/sm Small level of effort type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants