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 a separate build cache for each component of a package #3750

Merged
merged 1 commit into from
Jan 11, 2018

Conversation

tswelsh
Copy link
Contributor

@tswelsh tswelsh commented Jan 6, 2018

Note: Documentation fixes for https://docs.haskellstack.org/en/stable/ should target the "stable" branch, not master.

Please include the following checklist in your PR:

  • Any changes that could be relevant to users have been recorded in the ChangeLog.md
  • The documentation has been updated, if necessary.

Attempt at a fix for #3732. This introduces a separate build cache for each component of a package, instead of having one for the entire package.

Previously, if a package foo was built with tests enabled then the test files (say Spec.hs) would be recorded in the build cache. With no further source changes, if the dirtiness of foo is checked in a scenario where we are not asking for foo's tests (and so not mentioning Spec.hs at all), the dirtiness check of the source files would spot that Spec.hs is in the build cache but not in the files we want to compile, so a rebuild would be triggered. With this patch, we only compare with the build cache for the relevant component.

I think this is a better situation than before, but still not perfect - dirty files are not tracked on a component basis with this, so I think we can still get some useless rebuilds. However, querying dirty files by component looked a bit more involved, and would change much of the ConstructPlan module. Apart from it being more complicated, I wasn't sure if I would be stepping on @ezyang's toes with work in there.

Please also shortly describe how you tested your change. Bonus points for added tests!

Manual testing based on @snoyberg's example in the linked issue: a project with 2 packages, foo and bar that depends on foo, both with test suites.

@ezyang
Copy link

ezyang commented Jan 6, 2018

I haven't gotten around to understanding how build caches work in Stack, so no complaints form me :>

@tswelsh
Copy link
Contributor Author

tswelsh commented Jan 6, 2018

Maybe I don't understand either because this patch is broken :-) - my testing somehow failed to reveal that it doesn't rebuild if library source files change. Think it's easy enough to fix, but may as well hold of on reviewing until I sort it.

@tswelsh tswelsh force-pushed the 3732-recompilation branch from c1b3e3d to 5b18809 Compare January 7, 2018 23:18
@tswelsh
Copy link
Contributor Author

tswelsh commented Jan 7, 2018

I've rebased, hlinted and fixed the initial error, so it's ready to be looked over. The AppVeyor failure looks unrelated.

Copy link
Contributor

@snoyberg snoyberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks! I'm going to backport this to the stable branch as well.

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.

3 participants