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

Use IFD to reduce copySources invalidation surface #147

Closed

Conversation

exFalso
Copy link
Contributor

@exFalso exFalso commented Feb 10, 2021

This PR fixes copySources invalidating the two-stage build. The reasons for the invalidation:

  • copySourcesFrom was an attribute of the dummy-src derivation, causing invalidation if any sources changed
  • copySourcesFrom was used directly as the cp source directory with $copySourcesFrom/$p, invalidating the command script even if $p has not changed

The PR introduces the following changes:

  • $copySourcesFrom/$p changed to ${copySourcesFrom + "/${p}"}, and the surrounding for loop to a concatStrings. This causes the subdirectory to be treated like a proper nix path, so as long as copySourcesFrom is a local path, the script will only invalidate if $p changes.
  • An IFD indirection was introduced in the form of ifdForceCopy. This is to handle cases where copySourcesFrom is not a local path, but a /nix/store one (like in our use case). The indirection effectively "snips" the dependency chain, so it doesn't matter what derivation copySourcesFrom comes from and how it invalidates, ifdForceCopy will re-copy the subdirectory as a separate "leaf" store path. NOTE: this solution will effectively re-copy the sources twice. This could be reduced somewhat by using gzip instead of straight copying for the IFD.
  • copySourcesFrom was removed from dummy-src's attributes

@Patryk27
Copy link
Contributor

Hi, thanks for this merge request - unfortunately, unless there were any recent developments in this area, to keep naersk working with Hydra, we have to avoid using IFD; and so given the choice, I'd rather keep the current approach; thanks again!

@Patryk27 Patryk27 closed this Mar 28, 2022
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.

2 participants